diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index d3fe6b8c48..2cc8ec16ff 100644 --- a/include/clang/AST/Decl.h +++ b/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; @@ -1102,6 +1106,13 @@ public: /// a C++0x for-range statement. 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 diff --git a/lib/ARCMigrate/Transforms.cpp b/lib/ARCMigrate/Transforms.cpp index ab3bc1dbe7..0c9096296d 100644 --- a/lib/ARCMigrate/Transforms.cpp +++ b/lib/ARCMigrate/Transforms.cpp @@ -421,10 +421,7 @@ public: if (IsLV != Expr::MLV_ConstQualified) return true; VarDecl *var = cast(declRef->getDecl()); - if (var->getType().getLocalQualifiers().getObjCLifetime() - == Qualifiers::OCL_ExplicitNone && - (var->getTypeSourceInfo() && - !var->getTypeSourceInfo()->getType().isConstQualified())) { + if (var->isARCPseudoStrong()) { Transaction Trans(Pass.TA); if (Pass.TA.clearDiagnostic(diag::err_typecheck_arr_assign_enumeration, Exp->getOperatorLoc())) { diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp index 99eb0d386b..67f6531d79 100644 --- a/lib/AST/DeclObjC.cpp +++ b/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(); - // 'self' is always __strong, although as a special case we don't - // actually retain it except in init methods. + // 'self' is always __strong. It's actually pseudo-strong except + // 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())); diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index daa37eea8f..a79f031ec0 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/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); - EmitAutoVarWithLifetime(*this, D, loc, lifetime); + if (!D.isARCPseudoStrong()) { + 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() && - isa(D)) { + if (D.isARCPseudoStrong()) { const ObjCMethodDecl *method = cast(CurCodeDecl); assert(&D == method->getSelfDecl()); + assert(lt == Qualifiers::OCL_Strong); + assert(qs.hasConst()); assert(method->getMethodFamily() != OMF_init); (void) method; lt = Qualifiers::OCL_ExplicitNone; diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp index cdc2fffd94..ce9c262fd8 100644 --- a/lib/CodeGen/CGObjC.cpp +++ b/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(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(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. diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index dec7736bdc..f34a70c5d9 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/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); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 2d8ed17b70..e3fde7cfc9 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/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 - // infer const. + // In ARC, use some specialized diagnostics for occasions where we + // infer 'const'. These are always pseudo-strong variables. if (S.getLangOptions().ObjCAutoRefCount) { DeclRefExpr *declRef = dyn_cast(E->IgnoreParenCasts()); if (declRef && isa(declRef->getDecl())) { VarDecl *var = cast(declRef->getDecl()); - // If the variable wasn't written with 'const', there are some - // cases where we infer const anyway: - // - self - // - fast enumeration variables - if (!var->getTypeSourceInfo() || - !var->getTypeSourceInfo()->getType().isConstQualified()) { + // Use the normal diagnostic if it's pseudo-__strong but the + // user actually wrote 'const'. + if (var->isARCPseudoStrong() && + (!var->getTypeSourceInfo() || + !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); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index a5ad0a5a2b..9af9c8de1b 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/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 - // variable of a fast enumeration loop. Rather than actually - // trying to catch that during declaration processing, we - // remove the consequences here. - if (getLangOptions().ObjCAutoRefCount) { - SplitQualType split = var->getType().split(); + // foreach variables are never actually initialized in the way that + // the parser came up with. + var->setInit(0); - // Inferred lifetime will show up as a local qualifier because - // explicit lifetime would have shown up as an AttributedType - // instead. - if (split.second.hasObjCLifetime()) { - // Change the qualification to 'const __unsafe_unretained'. - split.second.setObjCLifetime(Qualifiers::OCL_ExplicitNone); - split.second.addConst(); - var->setType(Context.getQualifiedType(split.first, split.second)); - var->setInit(0); + // In ARC, we don't need to retain the iteration variable of a fast + // enumeration loop. Rather than actually trying to catch that + // during declaration processing, we remove the consequences here. + if (getLangOptions().ObjCAutoRefCount) { + QualType type = var->getType(); + + // Only do this if we inferred the lifetime. Inferred lifetime + // will show up as a local qualifier because explicit lifetime + // 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); } } } diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index fab2069bc5..24ab544dcd 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/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)); diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp index 7c24088c65..651e2078cf 100644 --- a/lib/Serialization/ASTWriterDecl.cpp +++ b/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(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 diff --git a/test/CodeGenObjC/arc-foreach.m b/test/CodeGenObjC/arc-foreach.m index 00b0adf393..e8a5b8851e 100644 --- a/test/CodeGenObjC/arc-foreach.m +++ b/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); - -@class NSString; -extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2))); +extern void use(id); +extern void use_block(void (^)(void)); @class NSArray; -int main (int argc, const char * argv[]) -{ - NSArray *array; - for ( NSString *str in array) { - dispatch_block_t blk = ^{ - NSLog(@"str in block: %@", str); - }; - blk(); - } - return 0; +void test0(NSArray *array) { + // 'x' should be initialized without a retain. + // We should actually do a non-constant capture, and that + // capture should require a retain. + for (id x in array) { + use_block(^{ use(x); }); + } } -// CHECK-LP64: define internal void @__main_block_invoke -// CHECK-LP64: [[BLOCK:%.*]] = bitcast i8* {{%.*}} to [[BLOCK_T:%.*]]* +// CHECK-LP64: define void @test0( +// 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: call void ([[OPAQUE_T]]*, ...)* @NSLog +// CHECK-LP64-NEXT: [[T2:%.*]] = load i8** [[T0]], align 8 +// 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]]) diff --git a/test/SemaObjC/arc.m b/test/SemaObjC/arc.m index 4c2ae236ac..d867b7332e 100644 --- a/test/SemaObjC/arc.m +++ b/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) {