[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:620
+                         SourceLocation Loc) {
+    auto &DefaultmapInfo = getTopOfStack().DefaultmapMap[DMVC];
+    DefaultmapInfo.ImplicitBehavior = DMIB;
----------------
`auto` -> to real type


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:626
+  bool hasSetDefaultmapCategory(OpenMPDefaultmapClauseKind VariableCategory) {
+    int VC = static_cast<int>(VariableCategory);
+    return getTopOfStack().DefaultmapMap[VC].ImplicitBehavior != DMIB_unspecified;
----------------
No need for explicit typecast, enums are implcitly casted to ints.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2120-2121
+            isDefaultmapDefaultBehaviorAtLevel(NewLevel, DMVC_pointer)) ||
+          (DSAStack->
+            isDefaultmapDefaultBehaviorAtLevel(NewLevel, DMVC_aggregate)))
         OMPC = OMPC_firstprivate;
----------------
Why aggregates are firstprivate by default? I believe only scalars are firstprivates by default.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3198
   }
-  ArrayRef<Expr *> getImplicitMap() const { return ImplicitMap; }
+  llvm::SmallVector<Expr *, 4>
+    getImplicitMap(DefaultMapVariableCategory DMVC) const {
----------------
Revert back to `ArrayRef`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4435-4437
+        if (DMIB == DMIB_alloc) Kind = OMPC_MAP_alloc;
+        else if (DMIB == DMIB_to) Kind = OMPC_MAP_to;
+        else if (DMIB == DMIB_from) Kind = OMPC_MAP_from;
----------------
Use `switch`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16349
 
+static DefaultMapImplicitBehavior DefaultmapModifierToImplicitBehavior(
+  OpenMPDefaultmapClauseModifier M) {
----------------
1. Function names must start from verb.
2. Functions must start from lower letter.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16372
+
+static DefaultMapVariableCategory DefaultmapKindToVariableCategory(
+  OpenMPDefaultmapClauseKind Kind) {
----------------
1. Function names must start from verb.
2. Functions must start from lower letter.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16411-16420
+    bool isDefaultmapModifier = (M == OMPC_DEFAULTMAP_MODIFIER_alloc) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_to) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_from) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_tofrom) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_firstprivate) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_default) ||
+                                (M == OMPC_DEFAULTMAP_MODIFIER_none);
----------------
Just `M != OMPC_DEFAULTMAP_MODIFIER_unknown` and `Kind != OMPC_DEFAULTMAP_unknown`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16426
+        Value += "'scalar', 'aggregate', 'pointer'";
+        Loc = KindLoc;
+        Diag(Loc, diag::err_omp_unexpected_clause_value)
----------------
Why need to add a new `Loc` var here? Use the required `KindLoc`\`MLoc` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204