[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, so these array-shaping expressions are only allowed as operands to specific OpenMP directives?  That's a plausible interpretation of "The shape-operator can appear only in clauses where it is explicitly allowed" from the spec <https://www.openmp.org/spec-html/5.0/openmpsu20.html>.  If it were a more general l-value expression, we could handle that just fine by building the type using `ConstantArrayType`/`VariableArrayType` as appropriate; but if the language intentionally restricts it, the placeholder approach seems fine.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:4748
+       Base->isInstantiationDependent() ||
+       Base->containsUnexpandedParameterPack()))
+    return OMPArrayShapingExpr::Create(Context, Context.DependentTy, Base,
----------------
Just check `isTypeDependent()`, none of these other conditions should interfere with type-checking.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:4751
+                                       LParenLoc, RParenLoc, Dims, Brackets);
+  if (!BaseTy->isAnyPointerType())
+    return ExprError(Diag(Base->getExprLoc(),
----------------
I think you should perform DefaultFunctionArrayLvalueConversion here so that e.g. arrays will decay to pointers, you load pointers from l-values, and so on.  If you do so, it'll handle placeholders for you.

Do you really want to allow this to operate on non-C pointer types?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:4758
+  for (Expr *Dim : Dims) {
+    if (Dim->getType()->isNonOverloadPlaceholderType()) {
+      ExprResult Result = CheckPlaceholderExpr(Dim);
----------------
I think overload placeholders need to be resolved here, too.  You may have copied this code from some different place that has the ability to resolve overloads later, but in this case that's not true.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:4771
+    }
+    if (!Dim->isValueDependent() && !Dim->isTypeDependent()) {
+      ExprResult Result =
----------------
You don't really care about value-dependence here, just type-dependence.  You can check value-dependence before doing the constant-evaluation check below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144