Establish the iteration variable of an ObjC for-in loop before

emitting the collection expression.  Fixes some really, really broken
code.

llvm-svn: 126193

clang/lib/CodeGen/CGDecl.cpp

@@ -782,10 +782,12 @@ static bool isCapturedBy(const VarDecl &var, const Expr *e) {
 }
 
 void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) {
+  assert(emission.Variable && "emission was not valid!");
+
   // If this was emitted as a global constant, we're done.
   if (emission.wasEmittedAsGlobal()) return;
 
-  const VarDecl &D = emission.Variable;
+  const VarDecl &D = *emission.Variable;
   QualType type = D.getType();
 
   // If this local has an initializer, emit it now.
@@ -940,10 +942,12 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) {
 }
 
 void CodeGenFunction::EmitAutoVarCleanups(const AutoVarEmission &emission) {
+  assert(emission.Variable && "emission was not valid!");
+
   // If this was emitted as a global constant, we're done.
   if (emission.wasEmittedAsGlobal()) return;
 
-  const VarDecl &D = emission.Variable;
+  const VarDecl &D = *emission.Variable;
 
   // Handle C++ destruction of variables.
   if (getLangOptions().CPlusPlus) {

clang/lib/CodeGen/CGObjC.cpp

@@ -663,6 +663,11 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
     return;
   }
 
+  // The local variable comes into scope immediately.
+  AutoVarEmission variable = AutoVarEmission::invalid();
+  if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement()))
+    variable = EmitAutoVarAlloca(*cast<VarDecl>(SD->getSingleDecl()));
+
   CGDebugInfo *DI = getDebugInfo();
   if (DI) {
     DI->setLocation(S.getSourceRange().getBegin());
@@ -799,22 +804,23 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
 
   // Initialize the element variable.
   RunCleanupsScope elementVariableScope(*this);
-  bool elementIsDecl;
+  bool elementIsVariable;
   LValue elementLValue;
   QualType elementType;
   if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement())) {
-    EmitStmt(SD);
-    const VarDecl* D = cast<VarDecl>(SD->getSingleDecl());
+    // Initialize the variable, in case it's a __block variable or something.
+    EmitAutoVarInit(variable);
 
+    const VarDecl* D = cast<VarDecl>(SD->getSingleDecl());
     DeclRefExpr tempDRE(const_cast<VarDecl*>(D), D->getType(),
                         VK_LValue, SourceLocation());
     elementLValue = EmitLValue(&tempDRE);
     elementType = D->getType();
-    elementIsDecl = true;
+    elementIsVariable = true;
   } else {
     elementLValue = LValue(); // suppress warning
     elementType = cast<Expr>(S.getElement())->getType();
-    elementIsDecl = false;
+    elementIsVariable = false;
   }
   const llvm::Type *convertedElementType = ConvertType(elementType);
 
@@ -837,11 +843,16 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
 
   // Make sure we have an l-value.  Yes, this gets evaluated every
   // time through the loop.
-  if (!elementIsDecl)
+  if (!elementIsVariable)
     elementLValue = EmitLValue(cast<Expr>(S.getElement()));
 
   EmitStoreThroughLValue(RValue::get(CurrentItem), elementLValue, elementType);
 
+  // If we do have an element variable, this assignment is the end of
+  // its initialization.
+  if (elementIsVariable)
+    EmitAutoVarCleanups(variable);
+
   // Perform the loop body, setting up break and continue labels.
   BreakContinueStack.push_back(BreakContinue(LoopEnd, AfterBody));
   {
@@ -891,7 +902,7 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
   // No more elements.
   EmitBlock(EmptyBB);
 
-  if (!elementIsDecl) {
+  if (!elementIsVariable) {
     // If the element was not a declaration, set it to be null.
 
     llvm::Value *null = llvm::Constant::getNullValue(convertedElementType);

clang/lib/CodeGen/CodeGenFunction.h

@@ -1560,7 +1560,7 @@ public:
   class AutoVarEmission {
     friend class CodeGenFunction;
 
-    const VarDecl &Variable;
+    const VarDecl *Variable;
 
     /// The alignment of the variable.
     CharUnits Alignment;
@@ -1578,13 +1578,18 @@ public:
     /// initializer.
     bool IsConstantAggregate;
 
+    struct Invalid {};
+    AutoVarEmission(Invalid) : Variable(0) {}
+
     AutoVarEmission(const VarDecl &variable)
-      : Variable(variable), Address(0), NRVOFlag(0),
+      : Variable(&variable), Address(0), NRVOFlag(0),
         IsByRef(false), IsConstantAggregate(false) {}
 
     bool wasEmittedAsGlobal() const { return Address == 0; }
 
   public:
+    static AutoVarEmission invalid() { return AutoVarEmission(Invalid()); }
+
     /// Returns the address of the object within this declaration.
     /// Note that this does not chase the forwarding pointer for
     /// __block decls.
@@ -1592,8 +1597,8 @@ public:
       if (!IsByRef) return Address;
 
       return CGF.Builder.CreateStructGEP(Address,
-                                         CGF.getByRefValueLLVMField(&Variable),
-                                         Variable.getNameAsString());
+                                         CGF.getByRefValueLLVMField(Variable),
+                                         Variable->getNameAsString());
     }
   };
   AutoVarEmission EmitAutoVarAlloca(const VarDecl &var);

clang/test/CodeGenObjC/for-in.m

@@ -42,3 +42,9 @@ void t1() {
       break;
   }
 }
+
+// rdar://problem/9027663
+void t2(NSArray *array) {
+  for (NSArray *array in array) { // expected-warning {{collection expression type 'NSArray *' may not respond}}
+  }
+}