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



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

Reply via email to