Fix rdar://6257721 by tightening up the block "snapshot" check, and

move it to its own predicate to make it more clear.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@57796 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index c8610da6bd9e89eb1dbf01dbe1041ec55f94c3b7..563718ccc8d53ebafee175c688f928005611a979 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -309,6 +309,36 @@ Sema::ActOnStringLiteral(const Token *StringToks, unsigned NumStringToks) {
                            StringToks[NumStringToks-1].getLocation());
 }
 
+/// ShouldSnapshotBlockValueReference - Return true if a reference inside of
+/// CurBlock to VD should cause it to be snapshotted (as we do for auto
+/// variables defined outside the block) or false if this is not needed (e.g.
+/// for values inside the block or for globals).
+///
+/// FIXME: This will create BlockDeclRefExprs for global variables,
+/// function references, etc which is suboptimal :) and breaks
+/// things like "integer constant expression" tests.
+static bool ShouldSnapshotBlockValueReference(BlockSemaInfo *CurBlock,
+                                              ValueDecl *VD) {
+  // If the value is defined inside the block, we couldn't snapshot it even if
+  // we wanted to.
+  if (CurBlock->TheDecl == VD->getDeclContext())
+    return false;
+  
+  // If this is an enum constant or function, it is constant, don't snapshot.
+  if (isa<EnumConstantDecl>(VD) || isa<FunctionDecl>(VD))
+    return false;
+
+  // If this is a reference to an extern, static, or global variable, no need to
+  // snapshot it.
+  // FIXME: What about 'const' variables in C++?
+  if (const VarDecl *Var = dyn_cast<VarDecl>(VD))
+    return Var->hasLocalStorage();
+  
+  return true;
+}  
+    
+
+
 /// ActOnIdentifierExpr - The parser read an identifier in expression context,
 /// validate it per-C99 6.5.1.  HasTrailingLParen indicates whether this
 /// identifier is used in a function call context.
@@ -397,16 +427,16 @@ Sema::ExprResult Sema::ActOnIdentifierExpr(Scope *S, SourceLocation Loc,
   // Only create DeclRefExpr's for valid Decl's.
   if (VD->isInvalidDecl())
     return true;
-    
-  // FIXME: This will create BlockDeclRefExprs for global variables,
-  // function references, etc which is suboptimal :) and breaks
-  // things like "integer constant expression" tests.
+  
+  // If the identifier reference is inside a block, and it refers to a value
+  // that is outside the block, create a BlockDeclRefExpr instead of a
+  // DeclRefExpr.  This ensures the value is treated as a copy-in snapshot when
+  // the block is formed.
   //
-  if (CurBlock && (CurBlock->TheDecl != VD->getDeclContext()) &&
-      !isa<EnumConstantDecl>(VD)) {
-    // If we are in a block and the variable is outside the current block,
-    // bind the variable reference with a BlockDeclRefExpr.
-    
+  // We do not do this for things like enum constants, global variables, etc,
+  // as they do not get snapshotted.
+  //
+  if (CurBlock && ShouldSnapshotBlockValueReference(CurBlock, VD)) {
     // The BlocksAttr indicates the variable is bound by-reference.
     if (VD->getAttr<BlocksAttr>())
       return new BlockDeclRefExpr(VD, VD->getType(), Loc, true);

diff --git a/test/Sema/block-misc.c b/test/Sema/block-misc.c
index 77de4a6c9e2cfe0fe57c4fade21cc008bdc2f660..62b774e26d41420c7702545aff2b209cb1fc3ed7 100644 (file)
--- a/test/Sema/block-misc.c
+++ b/test/Sema/block-misc.c
@@ -62,3 +62,11 @@ int test4(int argc) {  // rdar://6251437
   }();
   return 0;
 }
+
+
+// rdar://6257721 - reference to static/global is byref by default.
+static int test5g;
+void test5() {
+  bar(^{ test5g = 1; });
+}
+