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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: include/clang/Basic/Attr.td:3202
 
+def OMPDeclareVariantDecl : Attr {
+  let Spellings = [Pragma<"omp", "declare variant">];
----------------
For the first patch we don't need the attribute, just parsing/sema analysis


================
Comment at: include/clang/Basic/Attr.td:3220
+    private:
+      NamedDecl *VecVarNamedDecl;
+
----------------
This is definitely wrong, especially if we need to handle templates.


================
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)) {
----------------
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.


================
Comment at: lib/Parse/ParseOpenMP.cpp:838
+                                      CachedTokens &Toks, SourceLocation Loc) {
+  PP.EnterToken(Tok, /*IsReinject*/ true);
+  PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true,
----------------
`/*IsReinject=*/` 


================
Comment at: lib/Parse/ParseOpenMP.cpp:840
+  PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true,
+                      /*IsReinject*/ true);
+  // Consume the previously pushed token.
----------------
Same here


================
Comment at: lib/Sema/SemaOpenMP.cpp:4886
 
+Sema::DeclGroupPtrTy Sema::ActOnOpenMPDeclareVariantDirective(
+    DeclGroupPtrTy DG, IdentifierLoc &VectorVariantId, SourceRange SR) {
----------------
In this patch need to implement just basic checks, no need to create attributes, etc. I would suggest to look at the checks for Target Multiversioning attribite (see D40819)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294