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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to