[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2670-2676
+
+  // The next token may be an OpenMP pragma annotation token. That would
+  // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in
+  // this case, it came from an *attribute* rather than a pragma. Handle it now.
+  if (Tok.is(tok::annot_pragma_openmp_from_attr))
+    return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+
----------------
jdoerfert wrote:
> cor3ntin wrote:
> > The comment raises 2 questions: should it be called `annot_openmp_attr` instead? Does the comment describe what this does?
> > I imagine long terms attributes might be the ""normal"" scenario?
> > I imagine long terms attributes might be the ""normal"" scenario?
> 
> We can't assume that (C) and for C++ only time will tell.
FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to make that assumption some day :D

That said, the fact that these are called "PRAGMA_ANNOTATION" in TokenKinds.def seems misnamed to me anymore, which I think is the confusion.  It is a little strange that the `annot` is added automatically, but the `pragma` isn't... 

Either way, I think it is debatable what the `pragma` in these relates to.  My opinion is that it applies to the syntax (since the rest are #pragma SOMETHING), not that it is a `PRAGMA_ANNOTATION`, and I liked `annot_attr_openmp` to match `annot_pragma_openmp`, but I don't feel terribly strongly.  See our conversation on TokenKinds for the other half of this discussion.


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

https://reviews.llvm.org/D105648