[cfe-commits] r57796 - in /cfe/trunk: lib/Sema/SemaExpr.cpp test/Sema/block-misc.c

[Chris Lattner]

Author: lattner
Date: Mon Oct 20 00:16:36 2008
New Revision: 57796

URL: http://llvm.org/viewvc/llvm-project?rev=57796&view=rev
Log:
Fix rdar://6257721 by tightening up the block "snapshot" check, and
move it to its own predicate to make it more clear.

Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/block-misc.c

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=57796&r1=57795&r2=57796&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Oct 20 00:16:36 2008
@@ -309,6 +309,36 @@
                            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 @@
   // 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);

Modified: cfe/trunk/test/Sema/block-misc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/block-misc.c?rev=57796&r1=57795&r2=57796&view=diff

==============================================================================
--- cfe/trunk/test/Sema/block-misc.c (original)
+++ cfe/trunk/test/Sema/block-misc.c Mon Oct 20 00:16:36 2008
@@ -62,3 +62,11 @@
   }();
   return 0;
 }
+
+
+// rdar://6257721 - reference to static/global is byref by default.
+static int test5g;
+void test5() {
+  bar(^{ test5g = 1; });
+}
+