[PATCH] D98558: [OPENMP51]Initial support for the interop directive

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:7423-7424
+  /// preference-list.
+  /// \param IsTarget Uses the 'target' interop-type.
+  /// \param IsTargetSync Uses the 'targetsync' interop-type.
+  /// \param StartLoc Starting location of the clause.
----------------
Do we really need these params? I assume they are already part of `Exprs` though in `Expr *` form.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3098-3100
+        } else
+          SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
+                    StopBeforeMatch);
----------------
Braces here too or remove braces from the `if` substatement.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3135-3138
+      if (Tok.is(tok::comma))
+        ConsumeToken();
+      else
+        break;
----------------
```
if (!Tok.is(tok::comma))
  break;
ConsumeToken();
```


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3172
+  case OMPC_init:
+    Exprs.insert(Exprs.end(), Prefs.begin(), Prefs.end());
+    return Actions.ActOnOpenMPInitClause(Exprs, IsTarget, IsTargetSync, Loc,
----------------
`append`?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:14751
+  // string literals or constant integral expressions.
+  for (unsigned I = 1, End = Exprs.size(); I < End; ++I) {
+    const Expr *E = Exprs[I];
----------------
range-based loop:
```
for (const Expr *E : llvm::drop_begin(Exprs))
```


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

https://reviews.llvm.org/D98558