[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:1967
           (DSAStack->isForceCaptureByReferenceInTargetExecutable() &&
            !Ty->isAnyPointerType()) ||
           !Ty->isScalarType() ||
----------------
ABataev wrote:
> Seems to me, you're missing additional checks for pointers here.
Not done


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2830
+    return DMVC_pointer;
+  } else if (VD->getType()->isScalarType()) {
+    return DMVC_scalar;
----------------
No need `else if` here , just `if`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2832
+    return DMVC_scalar;
+  } else {
+    return DMVC_aggregate;
----------------
No need for `else` here, just `return`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3200-3201
   }
-  ArrayRef<Expr *> getImplicitMap() const { return ImplicitMap; }
+  std::array<llvm::SmallVector<Expr *, 4>, DMVC_unspecified> getImplicitMap()
+  const { return ImplicitMap; }
   const Sema::VarsWithInheritedDSAType &getVarsWithInheritedDSA() const {
----------------
Better to get each particular list via the `DMVC_` key passed as parameter


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4374
+  case DMIB_unspecified:
+    return OMPC_MAP_tofrom;
+  }
----------------
`OMPC_MAP_tofrom` must be returned only for `DMIB_tofrom`, for others, it must be `llvm_unreachable(....)`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16512
+    case OMPC_DEFAULTMAP_MODIFIER_default:
+      DSAStack->setDefaultDMAAttr(DMIB_default, DMVC_pointer, StartLoc);
+      break;
----------------
Just create 2 variables and initialize them. Call `setDefault...` only once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204