From: Chris Lattner <sabre@nondot.org>
Date: Mon, 20 Oct 2008 05:16:36 +0000 (+0000)
Subject: Fix rdar://6257721 by tightening up the block "snapshot" check, and
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=639e2d35d9881cefe167efa933e82ced3b0ed681;p=clang

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 c8610da6bd..563718ccc8 100644
--- 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 77de4a6c9e..62b774e26d 100644
--- 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; });
+}
+