[PATCH] D32372: Arrays of unknown bound in constant expressions

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

The change in direction from diagnosing the lvalue-to-rvalue conversion to diagnosing the pointer arithmetic seems fine to me (and is likely a better approach overall), but this means we should now treat a designator referring to element 0 of an array of unknown / runtime bound as being valid. We have minimal support for this already for `__builtin_object_size` evaluation, which you should be able to generalize to support arbitrary designators ending with such a value.



================
Comment at: lib/AST/ExprConstant.cpp:2957
+      else
+        return CompleteObject();
+    }
----------------
This path fails without producing a diagnostic (a default-constructed `CompleteObject()` is an error return). I think we should instead treat this as a valid `CompleteObject` with a type that simply can't be further decomposed.


================
Comment at: lib/AST/ExprConstant.cpp:5559-5567
+  // If we're dealing with an array of non-constant bound, the expression is
+  // not a constant expression. Use the Designator's most derived type field,
+  // since we may cover addition with a flexible array member.
+  if (!Info.checkingPotentialConstantExpression() && Result.Designator.Invalid
+   && !Result.Designator.MostDerivedType.isNull()
+   && Info.Ctx.getAsArrayType(Result.Designator.MostDerivedType))
+    CCEDiag(PExp, diag::note_constexpr_array_unknown_bound_arithmetic)
----------------
I think this should be handled in `SubobjectDesignator::adjustIndex` instead; there are other ways to get to the pointer arithmetic logic (such as array indexing and the implicit indexing we do in some builtins).


================
Comment at: lib/AST/ExprConstant.cpp:5679-5680
+        Result.addArray(Info, E, CAT);
+      else
+        Result.Designator.setInvalid();
+    }
----------------
We should never set a designator invalid without issuing a diagnostic. If you want to defer the diagnostic until pointer arithmetic happens, you need to be able to represent that situation in a valid designator. Perhaps generalizing the existing support for `isMostDerivedAnUnsizedArray` would be a path forward.


https://reviews.llvm.org/D32372