Objective-C fast enumeration loop variables are not retained in ARC, but

they should still be officially __strong for the purposes of errors, 
block capture, etc.  Make a new bit on variables, isARCPseudoStrong(),
and set this for 'self' and these enumeration-loop variables.  Change
the code that was looking for the old patterns to look for this bit,
and change IR generation to find this bit and treat the resulting         
variable as __unsafe_unretained for the purposes of init/destroy in
the two places it can come up.



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

include/clang/AST/Decl.h

@@ -702,8 +702,12 @@ private:
     /// \brief Whether this variable is the for-range-declaration in a C++0x
     /// for-range statement.
     unsigned CXXForRangeDecl : 1;
+
+    /// \brief Whether this variable is an ARC pseudo-__strong
+    /// variable;  see isARCPseudoStrong() for details.
+    unsigned ARCPseudoStrong : 1;
   };
-  enum { NumVarDeclBits = 13 }; // two reserved bits for now
+  enum { NumVarDeclBits = 13 }; // one reserved bit
 
   friend class ASTDeclReader;
   friend class StmtIteratorBase;
@@ -1103,6 +1107,13 @@ public:
   bool isCXXForRangeDecl() const { return VarDeclBits.CXXForRangeDecl; }
   void setCXXForRangeDecl(bool FRD) { VarDeclBits.CXXForRangeDecl = FRD; }
 
+  /// \brief Determine whether this variable is an ARC pseudo-__strong
+  /// variable.  A pseudo-__strong variable has a __strong-qualified
+  /// type but does not actually retain the object written into it.
+  /// Generally such variables are also 'const' for safety.
+  bool isARCPseudoStrong() const { return VarDeclBits.ARCPseudoStrong; }
+  void setARCPseudoStrong(bool ps) { VarDeclBits.ARCPseudoStrong = ps; }
+  
   /// \brief If this variable is an instantiated static data member of a
   /// class template specialization, returns the templated static data member
   /// from which it was instantiated.

lib/ARCMigrate/Transforms.cpp

@@ -421,10 +421,7 @@ public:
       if (IsLV != Expr::MLV_ConstQualified)
         return true;
       VarDecl *var = cast<VarDecl>(declRef->getDecl());
-      if (var->getType().getLocalQualifiers().getObjCLifetime()
+      if (var->isARCPseudoStrong()) {
-          == Qualifiers::OCL_ExplicitNone &&
-          (var->getTypeSourceInfo() &&
-           !var->getTypeSourceInfo()->getType().isConstQualified())) {
         Transaction Trans(Pass.TA);
         if (Pass.TA.clearDiagnostic(diag::err_typecheck_arr_assign_enumeration,
                                     Exp->getOperatorLoc())) {

lib/AST/DeclObjC.cpp

@@ -474,19 +474,22 @@ void ObjCMethodDecl::createImplicitParams(ASTContext &Context,
   } else // we have a factory method.
     selfTy = Context.getObjCClassType();
 
+  bool selfIsPseudoStrong = false;
   bool selfIsConsumed = false;
   if (isInstanceMethod() && Context.getLangOptions().ObjCAutoRefCount) {
     selfIsConsumed = hasAttr<NSConsumesSelfAttr>();
 
-    // 'self' is always __strong, although as a special case we don't
+    // 'self' is always __strong.  It's actually pseudo-strong except
-    // actually retain it except in init methods.
+    // in init methods, though.
     Qualifiers qs;
     qs.setObjCLifetime(Qualifiers::OCL_Strong);
     selfTy = Context.getQualifiedType(selfTy, qs);
 
     // In addition, 'self' is const unless this is an init method.
-    if (getMethodFamily() != OMF_init)
+    if (getMethodFamily() != OMF_init) {
       selfTy = selfTy.withConst();
+      selfIsPseudoStrong = true;
+    }
   }
 
   ImplicitParamDecl *self
@@ -497,6 +500,9 @@ void ObjCMethodDecl::createImplicitParams(ASTContext &Context,
   if (selfIsConsumed)
     self->addAttr(new (Context) NSConsumedAttr(SourceLocation(), Context));
 
+  if (selfIsPseudoStrong)
+    self->setARCPseudoStrong(true);
+
   setCmdDecl(ImplicitParamDecl::Create(Context, this, SourceLocation(),
                                        &Context.Idents.get("_cmd"),
                                        Context.getObjCSelType()));

lib/CodeGen/CGDecl.cpp

@@ -562,6 +562,37 @@ void CodeGenFunction::EmitScalarInit(const Expr *init,
   EmitStoreOfScalar(value, lvalue);
 }
 
+/// EmitScalarInit - Initialize the given lvalue with the given object.
+void CodeGenFunction::EmitScalarInit(llvm::Value *init, LValue lvalue) {
+  Qualifiers::ObjCLifetime lifetime = lvalue.getObjCLifetime();
+  if (!lifetime)
+    return EmitStoreThroughLValue(RValue::get(init), lvalue, lvalue.getType());
+
+  switch (lifetime) {
+  case Qualifiers::OCL_None:
+    llvm_unreachable("present but none");
+
+  case Qualifiers::OCL_ExplicitNone:
+    // nothing to do
+    break;
+
+  case Qualifiers::OCL_Strong:
+    init = EmitARCRetain(lvalue.getType(), init);
+    break;
+
+  case Qualifiers::OCL_Weak:
+    // Initialize and then skip the primitive store.
+    EmitARCInitWeak(lvalue.getAddress(), init);
+    return;
+
+  case Qualifiers::OCL_Autoreleasing:
+    init = EmitARCRetainAutorelease(lvalue.getType(), init);
+    break;
+  }
+
+  EmitStoreOfScalar(init, lvalue);  
+}
+
 /// canEmitInitWithFewStoresAfterMemset - Decide whether we can emit the
 /// non-zero parts of the specified initializer with equal or fewer than
 /// NumStores scalar stores.
@@ -995,8 +1026,10 @@ void CodeGenFunction::EmitAutoVarCleanups(const AutoVarEmission &emission) {
 
   if (Qualifiers::ObjCLifetime lifetime
         = D.getType().getQualifiers().getObjCLifetime()) {
-    llvm::Value *loc = emission.getObjectAddress(*this);
+    if (!D.isARCPseudoStrong()) {
-    EmitAutoVarWithLifetime(*this, D, loc, lifetime);
+      llvm::Value *loc = emission.getObjectAddress(*this);
+      EmitAutoVarWithLifetime(*this, D, loc, lifetime);
+    }
   }
 
   // Handle the cleanup attribute.
@@ -1081,10 +1114,11 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg,
 
       // 'self' is always formally __strong, but if this is not an
       // init method then we don't want to retain it.
-      if (lt == Qualifiers::OCL_Strong && qs.hasConst() &&
+      if (D.isARCPseudoStrong()) {
-          isa<ImplicitParamDecl>(D)) {
         const ObjCMethodDecl *method = cast<ObjCMethodDecl>(CurCodeDecl);
         assert(&D == method->getSelfDecl());
+        assert(lt == Qualifiers::OCL_Strong);
+        assert(qs.hasConst());
         assert(method->getMethodFamily() != OMF_init);
         (void) method;
         lt = Qualifiers::OCL_ExplicitNone;

lib/CodeGen/CGObjC.cpp

@@ -1110,6 +1110,9 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
     elementLValue = EmitLValue(&tempDRE);
     elementType = D->getType();
     elementIsVariable = true;
+
+    if (D->isARCPseudoStrong())
+      elementLValue.getQuals().setObjCLifetime(Qualifiers::OCL_ExplicitNone);
   } else {
     elementLValue = LValue(); // suppress warning
     elementType = cast<Expr>(S.getElement())->getType();
@@ -1136,10 +1139,12 @@ void CodeGenFunction::EmitObjCForCollectionStmt(const ObjCForCollectionStmt &S){
 
   // Make sure we have an l-value.  Yes, this gets evaluated every
   // time through the loop.
-  if (!elementIsVariable)
+  if (!elementIsVariable) {
     elementLValue = EmitLValue(cast<Expr>(S.getElement()));
-
+    EmitStoreThroughLValue(RValue::get(CurrentItem), elementLValue, elementType);
-  EmitStoreThroughLValue(RValue::get(CurrentItem), elementLValue, elementType);
+  } else {
+    EmitScalarInit(CurrentItem, elementLValue);
+  }
 
   // If we do have an element variable, this assignment is the end of
   // its initialization.

lib/CodeGen/CodeGenFunction.h

@@ -1597,6 +1597,7 @@ public:
 
   void EmitScalarInit(const Expr *init, const ValueDecl *D,
                       LValue lvalue, bool capturedByInit);
+  void EmitScalarInit(llvm::Value *init, LValue lvalue);
 
   typedef void SpecialInitFn(CodeGenFunction &Init, const VarDecl &D,
                              llvm::Value *Address);

lib/Sema/SemaExpr.cpp

@@ -8161,25 +8161,28 @@ static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
   case Expr::MLV_ConstQualified:
     Diag = diag::err_typecheck_assign_const;
 
-    // In ARC, use some specialized diagnostics for the times when we
+    // In ARC, use some specialized diagnostics for occasions where we
-    // infer const.
+    // infer 'const'.  These are always pseudo-strong variables.
     if (S.getLangOptions().ObjCAutoRefCount) {
       DeclRefExpr *declRef = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts());
       if (declRef && isa<VarDecl>(declRef->getDecl())) {
         VarDecl *var = cast<VarDecl>(declRef->getDecl());
 
-        // If the variable wasn't written with 'const', there are some
+        // Use the normal diagnostic if it's pseudo-__strong but the
-        // cases where we infer const anyway:
+        // user actually wrote 'const'.
-        //  - self
+        if (var->isARCPseudoStrong() &&
-        //  - fast enumeration variables
+            (!var->getTypeSourceInfo() ||
-        if (!var->getTypeSourceInfo() ||
+             !var->getTypeSourceInfo()->getType().isConstQualified())) {
-            !var->getTypeSourceInfo()->getType().isConstQualified()) {
+          // There are two pseudo-strong cases:
+          //  - self
           ObjCMethodDecl *method = S.getCurMethodDecl();
           if (method && var == method->getSelfDecl())
             Diag = diag::err_typecheck_arr_assign_self;
-          else if (var->getType().getObjCLifetime()
+
-                     == Qualifiers::OCL_ExplicitNone)
+          //  - fast enumeration variables
+          else
             Diag = diag::err_typecheck_arr_assign_enumeration;
+
           SourceRange Assign;
           if (Loc != OrigLoc)
             Assign = SourceRange(OrigLoc, OrigLoc);

lib/Sema/SemaStmt.cpp

@@ -71,22 +71,23 @@ void Sema::ActOnForEachDeclStmt(DeclGroupPtrTy dg) {
   // suppress any potential 'unused variable' warning.
   var->setUsed();
 
-  // In ARC, we don't want to lifetime for the iteration
+  // foreach variables are never actually initialized in the way that
-  // variable of a fast enumeration loop.  Rather than actually
+  // the parser came up with.
-  // trying to catch that during declaration processing, we
+  var->setInit(0);
-  // remove the consequences here.
-  if (getLangOptions().ObjCAutoRefCount) {
-    SplitQualType split = var->getType().split();
 
-    // Inferred lifetime will show up as a local qualifier because
+  // In ARC, we don't need to retain the iteration variable of a fast
-    // explicit lifetime would have shown up as an AttributedType
+  // enumeration loop.  Rather than actually trying to catch that
-    // instead.
+  // during declaration processing, we remove the consequences here.
-    if (split.second.hasObjCLifetime()) {
+  if (getLangOptions().ObjCAutoRefCount) {
-      // Change the qualification to 'const __unsafe_unretained'.
+    QualType type = var->getType();
-      split.second.setObjCLifetime(Qualifiers::OCL_ExplicitNone);
+
-      split.second.addConst();
+    // Only do this if we inferred the lifetime.  Inferred lifetime
-      var->setType(Context.getQualifiedType(split.first, split.second));
+    // will show up as a local qualifier because explicit lifetime
-      var->setInit(0);
+    // should have shown up as an AttributedType instead.
+    if (type.getLocalQualifiers().getObjCLifetime() == Qualifiers::OCL_Strong) {
+      // Add 'const' and mark the variable as pseudo-strong.
+      var->setType(type.withConst());
+      var->setARCPseudoStrong(true);
     }
   }
 }

lib/Serialization/ASTReaderDecl.cpp

@@ -710,6 +710,7 @@ void ASTDeclReader::VisitVarDecl(VarDecl *VD) {
   VD->VarDeclBits.ExceptionVar = Record[Idx++];
   VD->VarDeclBits.NRVOVariable = Record[Idx++];
   VD->VarDeclBits.CXXForRangeDecl = Record[Idx++];
+  VD->VarDeclBits.ARCPseudoStrong = Record[Idx++];
   if (Record[Idx++])
     VD->setInit(Reader.ReadExpr(F));
 

lib/Serialization/ASTWriterDecl.cpp

@@ -645,6 +645,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
   Record.push_back(D->isExceptionVariable());
   Record.push_back(D->isNRVOVariable());
   Record.push_back(D->isCXXForRangeDecl());
+  Record.push_back(D->isARCPseudoStrong());
   Record.push_back(D->getInit() ? 1 : 0);
   if (D->getInit())
     Writer.AddStmt(D->getInit());
@@ -670,7 +671,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
       D->RedeclLink.getNext() == D &&
       !D->hasCXXDirectInitializer() &&
       D->getInit() == 0 &&
-      !ParmVarDecl::classofKind(D->getKind()) &&
+      !isa<ParmVarDecl>(D) &&
       !SpecInfo)
     AbbrevToUse = Writer.getDeclVarAbbrev();
 
@@ -695,6 +696,8 @@ void ASTDeclWriter::VisitParmVarDecl(ParmVarDecl *D) {
     Writer.AddStmt(D->getUninstantiatedDefaultArg());
   Code = serialization::DECL_PARM_VAR;
 
+  assert(!D->isARCPseudoStrong()); // can be true of ImplicitParamDecl
+
   // If the assumptions about the DECL_PARM_VAR abbrev are true, use it.  Here
   // we dynamically check for the properties that we optimize for, but don't
   // know are true of all PARM_VAR_DECLs.
@@ -1426,6 +1429,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                       // isExceptionVariable
   Abv->Add(BitCodeAbbrevOp(0));                       // isNRVOVariable
   Abv->Add(BitCodeAbbrevOp(0));                       // isCXXForRangeDecl
+  Abv->Add(BitCodeAbbrevOp(0));                       // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(0));                       // HasInit
   Abv->Add(BitCodeAbbrevOp(0));                   // HasMemberSpecializationInfo
   // ParmVarDecl
@@ -1498,6 +1502,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isExceptionVariable
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNRVOVariable
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasInit
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasMemberSpecInfo
   // Type Source Info

test/CodeGenObjC/arc-foreach.m

@@ -1,27 +1,72 @@
 // RUN: %clang_cc1 -fblocks -fobjc-arc -fobjc-nonfragile-abi -triple x86_64-apple-darwin -O0 -emit-llvm %s -o %t-64.s
 // RUN: FileCheck -check-prefix LP64 --input-file=%t-64.s %s
 // rdar://9503326
+// rdar://9606600
 
-typedef void (^dispatch_block_t)(void);
+extern void use(id);
-
+extern void use_block(void (^)(void));
-@class NSString;
-extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2)));
 @class NSArray;
 
-int main (int argc, const char * argv[])
+void test0(NSArray *array) {
-{
+  // 'x' should be initialized without a retain.
-    NSArray *array;
+  // We should actually do a non-constant capture, and that
-    for ( NSString *str in array) {
+  // capture should require a retain.
-        dispatch_block_t blk = ^{
+  for (id x in array) {
-            NSLog(@"str in block: %@", str);
+    use_block(^{ use(x); });
-        };
+  }
-        blk();
-    }
-    return 0;
 }
 
-// CHECK-LP64: define internal void @__main_block_invoke
+// CHECK-LP64:    define void @test0(
-// CHECK-LP64: [[BLOCK:%.*]] = bitcast i8* {{%.*}} to [[BLOCK_T:%.*]]*
+// CHECK-LP64:      alloca [[ARRAY_T:%.*]]*,
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
+// CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
+// CHECK-LP64-NEXT: alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:%.*]],
+
+// CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[STATE_T]]* [[STATE]], i32 0, i32 1
+// CHECK-LP64-NEXT: [[T1:%.*]] = load i8*** [[T0]]
+// CHECK-LP64-NEXT: [[T2:%.*]] = getelementptr i8** [[T1]], i64
+// CHECK-LP64-NEXT: [[T3:%.*]] = load i8** [[T2]]
+// CHECK-LP64-NEXT: store i8* [[T3]], i8** [[X]]
+
+// CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
+// CHECK-LP64-NEXT: [[T1:%.*]] = load i8** [[X]]
+// CHECK-LP64-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]])
+// CHECK-LP64-NEXT: store i8* [[T2]], i8** [[T0]]
+// CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]] to void ()*
+// CHECK-LP64-NEXT: call void @use_block(void ()* [[T1]])
+// CHECK-LP64-NEXT: [[T1:%.*]] = load i8** [[T0]]
+// CHECK-LP64-NEXT: call void @objc_release(i8* [[T1]])
+
+// CHECK-LP64:    define internal void @__test0_block_invoke
+// CHECK-LP64:      [[BLOCK:%.*]] = bitcast i8* {{%.*}} to [[BLOCK_T]]*
 // CHECK-LP64-NEXT: [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
-// CHECK-LP64-NEXT: [[T2:%.*]] = load [[OPAQUE_T:%.*]]** [[T0]], align 8 
+// CHECK-LP64-NEXT: [[T2:%.*]] = load i8** [[T0]], align 8 
-// CHECK-LP64-NEXT: call void ([[OPAQUE_T]]*, ...)* @NSLog
+// CHECK-LP64-NEXT: call void @use(i8* [[T2]])
+
+void test1(NSArray *array) {
+  for (__weak id x in array) {
+    use_block(^{ use(x); });
+  }
+}
+
+// CHECK-LP64:    define void @test1(
+// CHECK-LP64:      alloca [[ARRAY_T:%.*]]*,
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
+// CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
+// CHECK-LP64-NEXT: alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:%.*]],
+
+// CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[STATE_T]]* [[STATE]], i32 0, i32 1
+// CHECK-LP64-NEXT: [[T1:%.*]] = load i8*** [[T0]]
+// CHECK-LP64-NEXT: [[T2:%.*]] = getelementptr i8** [[T1]], i64
+// CHECK-LP64-NEXT: [[T3:%.*]] = load i8** [[T2]]
+// CHECK-LP64-NEXT: call i8* @objc_initWeak(i8** [[X]], i8* [[T3]])
+
+// CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[BLOCK_T]]* [[BLOCK]], i32 0, i32 5
+// CHECK-LP64-NEXT: [[T1:%.*]] = call i8* @objc_loadWeak(i8** [[X]])
+// CHECK-LP64-NEXT: call i8* @objc_initWeak(i8** [[T0]], i8* [[T1]])
+// CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[BLOCK_T]]* [[BLOCK]] to void ()*
+// CHECK-LP64-NEXT: call void @use_block(void ()* [[T1]])
+// CHECK-LP64-NEXT: call void @objc_destroyWeak(i8** [[T0]])
+// CHECK-LP64-NEXT: call void @objc_destroyWeak(i8** [[X]])

test/SemaObjC/arc.m

@@ -241,7 +241,7 @@ id test9(Test9 *v) {
 // Test that the inference rules are different for fast enumeration variables.
 void test10(id collection) {
   for (id x in collection) {
-    __strong id *ptr = &x; // expected-error {{initializing '__strong id *' with an expression of type 'const __unsafe_unretained id *' changes retain/release properties of pointer}}
+    __strong id *ptr = &x; // expected-warning {{initializing '__strong id *' with an expression of type 'const __strong id *' discards qualifiers}}
   }
 
   for (__strong id x in collection) {