[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar marked 2 inline comments as done.
andwar added inline comments.


================
Comment at: lib/Parse/ParseOpenMP.cpp:779-783
+  auto Identinfo = PP.getIdentifierInfo(VectorVariantId.Ident->getName());
+  LookupResult Lookup(Actions, Identinfo, VectorVariantId.Loc,
+                      Sema::LookupOrdinaryName);
+  CXXScopeSpec SS;
+  if (!Actions.LookupParsedName(Lookup, getCurScope(), &SS)) {
----------------
ABataev wrote:
> andwar wrote:
> > ABataev wrote:
> > > 1. All this lookup must be performed in Sema.
> > > 2. How do you want to handle templates?
> > > 3. Why just do not parse it as an expression? And rely on existing parsing/sema analysis for expressions.
> > 1. That's fine, let me move it there.
> > 
> > 2. TL;DR We're not handling templates yet. 
> > 
> > 3. How about switching to `ParseUnqualifiedId` instead? That will still require going through `LookupParsedName` though.
> > 
> > Re 2. What the interaction between templates and #pragma(s) should be? The C++ standard doesn't specify that, but IMHO mixing '#pragma omp declare variant' and templates requires that interaction to be clearly defined. 
> > 
> > I know that OpenMP 5 allows 'variant-func-id' to be a template-id, but I couldn't find an example of any attribute that would use C++ template parameters. Also, isn't parsing of attributes and templates _orthogonal_ in clang (i.e. happening at completely different points during parsing)?
> > 
> > In any case, we felt that support for 'template-id' could come later. How do you feel about it?
> No, don't try to parse function names directly. What about `::fun` format? Or `std::fun`? Your solution does not support it.
> I have basic implementation that handles all these cases. Let me commit it, because we need it to support other context selectors. It is very similar to your solution but handles all the problems. We just don't have time to wait, need to support other context selectors.
That's true - my approach doesn't support `::fun` or `std::fun`. The idea is to fill in the gaps as we progress later. 

@ABataev, If you have a more generic implementation  then it makes a lot of sense to go with that instead. Please, feel free to re-use this patch if that helps.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67294/new/

https://reviews.llvm.org/D67294