[PATCH] D21187: Allow use of lambda expressions in array bounds

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

Sorry for my late reply.


================
Comment at: lib/Sema/SemaExpr.cpp:12825
@@ -12824,1 +12824,3 @@
+      // BlockContext.
+      } else if (!Rec.IsArrayBound) {
         // C++1y [expr.const]p2:
----------------
rsmith wrote:
> This isn't correct; you still need to produce the diagnostic even if we're in an array bound, but it should be an `ExtWarn` controlled by `-Wvla`. A case like
> 
>     void f() {
>       int arr[ true ? 1 : []{return 0}() ];
>     }
> 
> is ill-formed in standard C++, but as we can evaluate the array bound as a constant, Clang will no longer diagnose it with this change in place.
Is this ill-formed too in standard c++?

int a[true ? 2 : !dynamic_cast<S2*>(s1) + 1];

clang doesn't warn or error out when compiling the expression above.


================
Comment at: lib/Sema/SemaExpr.cpp:12848-12853
@@ -12846,8 +12847,8 @@
   if (Rec.isUnevaluated() || Rec.Context == ConstantEvaluated) {
     ExprCleanupObjects.erase(ExprCleanupObjects.begin() + Rec.NumCleanupObjects,
                              ExprCleanupObjects.end());
     ExprNeedsCleanups = Rec.ParentNeedsCleanups;
     CleanupVarDeclMarking();
     std::swap(MaybeODRUseExprs, Rec.SavedMaybeODRUseExprs);
   // Otherwise, merge the contexts together.
   } else {
----------------
rsmith wrote:
> This also looks wrong for your lambda-in-VLA-bound case.
> 
> Perhaps the best thing to do is to check whether we have a VLA *before* we pop the ExpressionEvaluationContextRecord, and if so, convert the context from ConstantEvaluated to Evaluated.
It seems like we won't be able to warn about lambdas in constant expressions at all if we change the context before popping ExpressionEvaluationContextRecord, so there would be no warnings for your example code (ternary expression used for array bound). Is that your intention?


http://reviews.llvm.org/D21187