[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 24 13:43:34 PST 2020


aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Parse/Parser.h:2959
+  /// if there was an error.
+  bool ParseOMPDeclareVariantMatchClause(SourceLocation Loc, OMPTraitInfo &TI);
+
----------------
This file is inconsistent currently, but it looks like `parseOMP...` is the current "winner" for the local style (and matches the coding standard). Feel free to rename the other OMP parse functions to be `parse` instead of `Parse` as a separate NFC patch, if you'd like.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1354
+    ConsumeAnyToken();
+  if (DeclVarData.hasValue() && !TI.Sets.empty())
+    Actions.ActOnOpenMPDeclareVariantDirective(
----------------
You can drop the `hasValue()` and just rely on the conversion to bool operator.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1356
+    Actions.ActOnOpenMPDeclareVariantDirective(
+        DeclVarData.getValue().first, DeclVarData.getValue().second, TI,
+        SourceRange(Loc, Tok.getLocation()));
----------------
You can replace the `getValue()` calls with `->`.


================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1775
+
+    bool IsApplicableOpenMPSelector = isVariantApplicableInContext(VMI, OMPCtx);
+    if (IsApplicableOpenMPSelector)
----------------
No real value from using this local only once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74941





More information about the llvm-commits mailing list