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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
