ahatanak created this revision.
ahatanak added a subscriber: cfe-commits.

Currently, when clang compiles the following code,

id test() {
  return @{@"a": [](){}, @"b": [](){}};
}

it builds an AST that is incorrect:

ReturnStmt 0x10d080448
`-ExprWithCleanups 0x10d080428
    |-cleanup Block 0x10d0801f0 // points to the second BlockDecl
      ...
      -BlockDecl 0x10d07f150 // First block
      ...
      -BlockDecl 0x10d0801f0 // Second block
      ...
        `-ExprWithCleanups 0x10d0801d0
           |-cleanup Block 0x10d07f150 // points to the first BlockDecl

The ExprWithCleanups for the ReturnStmt has only one block (the second block) 
in its cleanup list. The other block (the first block) is in the cleanup list 
of the ExprWithCleanups attached to the second block. This happens because 
Sema::ExprCleanupObjects is false when MaybeCreateExprWithCleanups is called in 
Sema::ActOnFinishFullExpr the first time, but is true when the function is 
called the second time (Sema::BuildBlockForLambdaConversion sets 
ExprCleanupObjects to true when it builds the block for the first lambda).

To fix this bug, this patch pushes a new evaluation context before calling 
BuildBlockForLambdaConversion. This ensures that Sema::ExprCleanupObjects is 
false when MaybeCreateExprWithCleanups is called the second time too.

http://reviews.llvm.org/D18815

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaObjCXX/block-cleanup.mm

Index: test/SemaObjCXX/block-cleanup.mm
===================================================================
--- /dev/null
+++ test/SemaObjCXX/block-cleanup.mm
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -std=gnu++11 -o 
/dev/null -x objective-c++ -fblocks -ast-dump %s 2>&1 | FileCheck %s
+
+// CHECK:      -FunctionDecl {{.*}} test 'id (void)'
+// CHECK-NEXT:   -CompoundStmt
+// CHECK-NEXT:     -ReturnStmt
+// CHECK-NEXT:       -ExprWithCleanups
+// CHECK-NEXT:         -cleanup Block
+// CHECK-NEXT:         -cleanup Block
+
+@interface NSDictionary
++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys 
count:(unsigned long)cnt;
+@end
+
+id test() {
+  return @{@"a": [](){}, @"b": [](){}};
+}
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6233,9 +6233,12 @@
       // follows the normal lifetime rules for block literals instead of being
       // autoreleased.
       DiagnosticErrorTrap Trap(Diags);
+      PushExpressionEvaluationContext(PotentiallyEvaluated);
       ExprResult Exp = BuildBlockForLambdaConversion(E->getExprLoc(),
                                                      E->getExprLoc(),
                                                      Method, E);
+      PopExpressionEvaluationContext();
+
       if (Exp.isInvalid())
         Diag(E->getExprLoc(), diag::note_lambda_to_block_conv);
       return Exp;


Index: test/SemaObjCXX/block-cleanup.mm
===================================================================
--- /dev/null
+++ test/SemaObjCXX/block-cleanup.mm
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -std=gnu++11 -o /dev/null -x objective-c++ -fblocks -ast-dump %s 2>&1 | FileCheck %s
+
+// CHECK:      -FunctionDecl {{.*}} test 'id (void)'
+// CHECK-NEXT:   -CompoundStmt
+// CHECK-NEXT:     -ReturnStmt
+// CHECK-NEXT:       -ExprWithCleanups
+// CHECK-NEXT:         -cleanup Block
+// CHECK-NEXT:         -cleanup Block
+
+@interface NSDictionary
++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
+@end
+
+id test() {
+  return @{@"a": [](){}, @"b": [](){}};
+}
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6233,9 +6233,12 @@
       // follows the normal lifetime rules for block literals instead of being
       // autoreleased.
       DiagnosticErrorTrap Trap(Diags);
+      PushExpressionEvaluationContext(PotentiallyEvaluated);
       ExprResult Exp = BuildBlockForLambdaConversion(E->getExprLoc(),
                                                      E->getExprLoc(),
                                                      Method, E);
+      PopExpressionEvaluationContext();
+
       if (Exp.isInvalid())
         Diag(E->getExprLoc(), diag::note_lambda_to_block_conv);
       return Exp;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to