From: John McCall Date: Tue, 13 Sep 2011 18:31:23 +0000 (+0000) Subject: Refactoring, mostly to give ObjCPropertyDecls stronger invariants for X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=265941bc308d65cc270d5c4de5806f37ce405606;p=clang Refactoring, mostly to give ObjCPropertyDecls stronger invariants for their semantic attributes and then to take advantage of that. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@139615 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h index 1af0b924cf..618fe367cf 100644 --- a/include/clang/AST/DeclObjC.h +++ b/include/clang/AST/DeclObjC.h @@ -1495,6 +1495,17 @@ public: return (PropertyAttributes & OBJC_PR_readonly); } + /// isAtomic - Return true if the property is atomic. + bool isAtomic() const { + return (PropertyAttributes & OBJC_PR_atomic); + } + + /// isRetaining - Return true if the property retains its value. + bool isRetaining() const { + return (PropertyAttributes & + (OBJC_PR_retain | OBJC_PR_strong | OBJC_PR_copy)); + } + /// getSetterKind - Return the method used for doing assignment in /// the property setter. This is only valid if the property has been /// defined to have a setter. diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 0a0fea3868..186908252f 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2886,7 +2886,7 @@ def err_arc_assign_property_ownership : Error< "existing ivar %1 for property %0 with %select{unsafe_unretained| assign}2 " "attribute must be __unsafe_unretained">; def err_arc_inconsistent_property_ownership : Error< - "%select{strong|weak|unsafe_unretained}1 property %0 may not also be " + "%select{|unsafe_unretained|strong|weak}1 property %0 may not also be " "declared %select{|__unsafe_unretained|__strong|__weak|__autoreleasing}2">; def err_arc_atomic_ownership : Error< "cannot perform atomic operation on a pointer to type %0: type has " diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp index 1572b4b3cf..8f5e1fb761 100644 --- a/lib/CodeGen/CGObjC.cpp +++ b/lib/CodeGen/CGObjC.cpp @@ -443,10 +443,10 @@ namespace { PropertyImplStrategy::PropertyImplStrategy(CodeGenModule &CGM, const ObjCPropertyImplDecl *propImpl) { const ObjCPropertyDecl *prop = propImpl->getPropertyDecl(); - ObjCPropertyDecl::PropertyAttributeKind attrs = prop->getPropertyAttributes(); + ObjCPropertyDecl::SetterKind setterKind = prop->getSetterKind(); - IsCopy = (attrs & ObjCPropertyDecl::OBJC_PR_copy); - IsAtomic = !(attrs & ObjCPropertyDecl::OBJC_PR_nonatomic); + IsCopy = (setterKind == ObjCPropertyDecl::Copy); + IsAtomic = prop->isAtomic(); HasStrong = false; // doesn't matter here. // Evaluate the ivar's size and alignment. @@ -456,14 +456,14 @@ PropertyImplStrategy::PropertyImplStrategy(CodeGenModule &CGM, = CGM.getContext().getTypeInfoInChars(ivarType); // If we have a copy property, we always have to use getProperty/setProperty. + // TODO: we could actually use setProperty and an expression for non-atomics. if (IsCopy) { Kind = GetSetProperty; return; } - // Handle retain/strong. - if (attrs & (ObjCPropertyDecl::OBJC_PR_retain - | ObjCPropertyDecl::OBJC_PR_strong)) { + // Handle retain. + if (setterKind == ObjCPropertyDecl::Retain) { // In GC-only, there's nothing special that needs to be done. if (CGM.getLangOptions().getGC() == LangOptions::GCOnly) { // fallthrough @@ -663,9 +663,8 @@ CodeGenFunction::generateObjCGetterBody(const ObjCImplementationDecl *classImpl, args.add(RValue::get(self), getContext().getObjCIdType()); args.add(RValue::get(cmd), getContext().getObjCSelType()); args.add(RValue::get(ivarOffset), getContext().getPointerDiffType()); - - assert(strategy.isAtomic()); - args.add(RValue::get(Builder.getTrue()), getContext().BoolTy); + args.add(RValue::get(Builder.getInt1(strategy.isAtomic())), + getContext().BoolTy); // FIXME: We shouldn't need to get the function info here, the // runtime already should have computed it to build the function. diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index b51e8313bd..52a20798e7 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3862,10 +3862,7 @@ static bool findRetainCycleOwner(Expr *e, RetainCycleOwner &owner) { const ObjCPropertyRefExpr *pre = cast->getSubExpr()->getObjCProperty(); if (pre->isImplicitProperty()) return false; ObjCPropertyDecl *property = pre->getExplicitProperty(); - if (!(property->getPropertyAttributes() & - (ObjCPropertyDecl::OBJC_PR_retain | - ObjCPropertyDecl::OBJC_PR_copy | - ObjCPropertyDecl::OBJC_PR_strong)) && + if (!property->isRetaining() && !(property->getPropertyIvarDecl() && property->getPropertyIvarDecl()->getType() .getObjCLifetime() == Qualifiers::OCL_Strong)) diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index fe5fc1a5a3..fb0d13a66a 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -24,6 +24,37 @@ using namespace clang; // Grammar actions. //===----------------------------------------------------------------------===// +/// getImpliedARCOwnership - Given a set of property attributes and a +/// type, infer an expected lifetime. The type's ownership qualification +/// is not considered. +/// +/// Returns OCL_None if the attributes as stated do not imply an ownership. +/// Never returns OCL_Autoreleasing. +static Qualifiers::ObjCLifetime getImpliedARCOwnership( + ObjCPropertyDecl::PropertyAttributeKind attrs, + QualType type) { + // retain, strong, copy, weak, and unsafe_unretained are only legal + // on properties of retainable pointer type. + if (attrs & (ObjCPropertyDecl::OBJC_PR_retain | + ObjCPropertyDecl::OBJC_PR_strong | + ObjCPropertyDecl::OBJC_PR_copy)) { + return Qualifiers::OCL_Strong; + } else if (attrs & ObjCPropertyDecl::OBJC_PR_weak) { + return Qualifiers::OCL_Weak; + } else if (attrs & ObjCPropertyDecl::OBJC_PR_unsafe_unretained) { + return Qualifiers::OCL_ExplicitNone; + } + + // assign can appear on other types, so we have to check the + // property type. + if (attrs & ObjCPropertyDecl::OBJC_PR_assign && + type->isObjCRetainableType()) { + return Qualifiers::OCL_ExplicitNone; + } + + return Qualifiers::OCL_None; +} + /// Check the internal consistency of a property declaration. static void checkARCPropertyDecl(Sema &S, ObjCPropertyDecl *property) { if (property->isInvalidDecl()) return; @@ -36,26 +67,23 @@ static void checkARCPropertyDecl(Sema &S, ObjCPropertyDecl *property) { // Nothing to do if we don't have a lifetime. if (propertyLifetime == Qualifiers::OCL_None) return; - Qualifiers::ObjCLifetime expectedLifetime; - unsigned selector; - - // Strong properties should have either strong or no lifetime. - if (propertyKind & (ObjCPropertyDecl::OBJC_PR_retain | - ObjCPropertyDecl::OBJC_PR_strong | - ObjCPropertyDecl::OBJC_PR_copy)) { - expectedLifetime = Qualifiers::OCL_Strong; - selector = 0; - } else if (propertyKind & ObjCPropertyDecl::OBJC_PR_weak) { - expectedLifetime = Qualifiers::OCL_Weak; - selector = 1; - } else if (propertyKind & (ObjCPropertyDecl::OBJC_PR_assign | - ObjCPropertyDecl::OBJC_PR_unsafe_unretained) && - property->getType()->isObjCRetainableType()) { - expectedLifetime = Qualifiers::OCL_ExplicitNone; - selector = 2; - } else { + Qualifiers::ObjCLifetime expectedLifetime + = getImpliedARCOwnership(propertyKind, property->getType()); + if (!expectedLifetime) { // We have a lifetime qualifier but no dominating property - // attribute. That's okay. + // attribute. That's okay, but restore reasonable invariants by + // setting the property attribute according to the lifetime + // qualifier. + ObjCPropertyDecl::PropertyAttributeKind attr; + if (propertyLifetime == Qualifiers::OCL_Strong) { + attr = ObjCPropertyDecl::OBJC_PR_strong; + } else if (propertyLifetime == Qualifiers::OCL_Weak) { + attr = ObjCPropertyDecl::OBJC_PR_weak; + } else { + assert(propertyLifetime == Qualifiers::OCL_ExplicitNone); + attr = ObjCPropertyDecl::OBJC_PR_unsafe_unretained; + } + property->setPropertyAttributes(attr); return; } @@ -65,7 +93,7 @@ static void checkARCPropertyDecl(Sema &S, ObjCPropertyDecl *property) { S.Diag(property->getLocation(), diag::err_arc_inconsistent_property_ownership) << property->getDeclName() - << selector + << expectedLifetime << propertyLifetime; } @@ -393,9 +421,10 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, if (isAssign) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_assign); + // In the semantic attributes, one of nonatomic or atomic is always set. if (Attributes & ObjCDeclSpec::DQ_PR_nonatomic) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_nonatomic); - else if (Attributes & ObjCDeclSpec::DQ_PR_atomic) + else PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_atomic); // 'unsafe_unretained' is alias for 'assign'. @@ -417,84 +446,49 @@ static void checkARCPropertyImpl(Sema &S, SourceLocation propertyImplLoc, ObjCIvarDecl *ivar) { if (property->isInvalidDecl() || ivar->isInvalidDecl()) return; - QualType propertyType = property->getType(); - Qualifiers::ObjCLifetime propertyLifetime = propertyType.getObjCLifetime(); - ObjCPropertyDecl::PropertyAttributeKind propertyKind - = property->getPropertyAttributes(); - QualType ivarType = ivar->getType(); Qualifiers::ObjCLifetime ivarLifetime = ivarType.getObjCLifetime(); - - // Case 1: strong properties. - if (propertyLifetime == Qualifiers::OCL_Strong || - (propertyKind & (ObjCPropertyDecl::OBJC_PR_retain | - ObjCPropertyDecl::OBJC_PR_strong | - ObjCPropertyDecl::OBJC_PR_copy))) { - switch (ivarLifetime) { - case Qualifiers::OCL_Strong: - // Okay. - return; - - case Qualifiers::OCL_None: - case Qualifiers::OCL_Autoreleasing: - // These aren't valid lifetimes for object ivars; don't diagnose twice. - return; - - case Qualifiers::OCL_ExplicitNone: - case Qualifiers::OCL_Weak: - S.Diag(propertyImplLoc, diag::err_arc_strong_property_ownership) - << property->getDeclName() - << ivar->getDeclName() - << ivarLifetime; - break; - } - // Case 2: weak properties. - } else if (propertyLifetime == Qualifiers::OCL_Weak || - (propertyKind & ObjCPropertyDecl::OBJC_PR_weak)) { - switch (ivarLifetime) { - case Qualifiers::OCL_Weak: - // Okay. - return; - - case Qualifiers::OCL_None: - case Qualifiers::OCL_Autoreleasing: - // These aren't valid lifetimes for object ivars; don't diagnose twice. - return; - - case Qualifiers::OCL_ExplicitNone: - case Qualifiers::OCL_Strong: - S.Diag(propertyImplLoc, diag::error_weak_property) - << property->getDeclName() - << ivar->getDeclName(); - break; - } + // The lifetime implied by the property's attributes. + Qualifiers::ObjCLifetime propertyLifetime = + getImpliedARCOwnership(property->getPropertyAttributes(), + property->getType()); - // Case 3: assign properties. - } else if ((propertyKind & ObjCPropertyDecl::OBJC_PR_assign) && - propertyType->isObjCRetainableType()) { - switch (ivarLifetime) { - case Qualifiers::OCL_ExplicitNone: - // Okay. - return; - - case Qualifiers::OCL_None: - case Qualifiers::OCL_Autoreleasing: - // These aren't valid lifetimes for object ivars; don't diagnose twice. - return; - - case Qualifiers::OCL_Weak: - case Qualifiers::OCL_Strong: - S.Diag(propertyImplLoc, diag::err_arc_assign_property_ownership) - << property->getDeclName() - << ivar->getDeclName() - << ((property->getPropertyAttributesAsWritten() - & ObjCPropertyDecl::OBJC_PR_assign) != 0); - break; - } + // We're fine if they match. + if (propertyLifetime == ivarLifetime) return; - // Any other property should be ignored. - } else { + // These aren't valid lifetimes for object ivars; don't diagnose twice. + if (ivarLifetime == Qualifiers::OCL_None || + ivarLifetime == Qualifiers::OCL_Autoreleasing) + return; + + switch (propertyLifetime) { + case Qualifiers::OCL_Strong: + S.Diag(propertyImplLoc, diag::err_arc_strong_property_ownership) + << property->getDeclName() + << ivar->getDeclName() + << ivarLifetime; + break; + + case Qualifiers::OCL_Weak: + S.Diag(propertyImplLoc, diag::error_weak_property) + << property->getDeclName() + << ivar->getDeclName(); + break; + + case Qualifiers::OCL_ExplicitNone: + S.Diag(propertyImplLoc, diag::err_arc_assign_property_ownership) + << property->getDeclName() + << ivar->getDeclName() + << ((property->getPropertyAttributesAsWritten() + & ObjCPropertyDecl::OBJC_PR_assign) != 0); + break; + + case Qualifiers::OCL_Autoreleasing: + llvm_unreachable("properties cannot be autoreleasing"); + + case Qualifiers::OCL_None: + // Any other property should be ignored. return; } @@ -593,64 +587,55 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, PropertyIvar = PropertyId; ObjCPropertyDecl::PropertyAttributeKind kind = property->getPropertyAttributes(); - QualType PropType = Context.getCanonicalType(property->getType()); - - if ((kind & ObjCPropertyDecl::OBJC_PR_weak) && - !getLangOptions().ObjCAutoRefCount && + QualType PropType = property->getType(); + + QualType PropertyIvarType = PropType.getNonReferenceType(); + + // Add GC __weak to the ivar type if the property is weak. + if ((kind & ObjCPropertyDecl::OBJC_PR_weak) && getLangOptions().getGC() != LangOptions::NonGC) { - if (PropType.isObjCGCStrong()) { - Diag(PropertyLoc, - diag::err_gc_weak_property_strong_type); - Diag(property->getLocation(), diag::note_property_declare); + assert(!getLangOptions().ObjCAutoRefCount); + if (PropertyIvarType.isObjCGCStrong()) { + Diag(PropertyLoc, diag::err_gc_weak_property_strong_type); + Diag(property->getLocation(), diag::note_property_declare); + } else { + PropertyIvarType = + Context.getObjCGCQualType(PropertyIvarType, Qualifiers::Weak); } - else - PropType = Context.getObjCGCQualType(PropType, Qualifiers::Weak); } - QualType PropertyIvarType = PropType; - if (PropType->isReferenceType()) - PropertyIvarType = cast(PropType)->getPointeeType(); + // Check that this is a previously declared 'ivar' in 'IDecl' interface ObjCInterfaceDecl *ClassDeclared; Ivar = IDecl->lookupInstanceVariable(PropertyIvar, ClassDeclared); if (!Ivar) { - // In ARC, give the ivar a lifetime qualifier based on its + // In ARC, give the ivar a lifetime qualifier based on the // property attributes. if (getLangOptions().ObjCAutoRefCount && - !PropertyIvarType.getObjCLifetime()) { + !PropertyIvarType.getObjCLifetime() && + PropertyIvarType->isObjCRetainableType()) { + // It's an error if we have to do this and the user didn't + // explicitly write an ownership attribute on the property. if (!property->hasWrittenStorageAttribute() && - property->getType()->isObjCRetainableType() && - !(kind & ObjCPropertyDecl::OBJC_PR_strong) ) { + !(kind & ObjCPropertyDecl::OBJC_PR_strong)) { Diag(PropertyLoc, diag::err_arc_objc_property_default_assign_on_object); Diag(property->getLocation(), diag::note_property_declare); - } + } else { + Qualifiers::ObjCLifetime lifetime = + getImpliedARCOwnership(kind, PropertyIvarType); + assert(lifetime && "no lifetime for property?"); - // retain/copy have retaining lifetime. - if (kind & (ObjCPropertyDecl::OBJC_PR_retain | - ObjCPropertyDecl::OBJC_PR_strong | - ObjCPropertyDecl::OBJC_PR_copy)) { - Qualifiers qs; - qs.addObjCLifetime(Qualifiers::OCL_Strong); - PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs); - } - else if (kind & ObjCPropertyDecl::OBJC_PR_weak) { - if (!getLangOptions().ObjCRuntimeHasWeak) { + if (lifetime == Qualifiers::OCL_Weak && + !getLangOptions().ObjCRuntimeHasWeak) { Diag(PropertyLoc, diag::err_arc_weak_no_runtime); Diag(property->getLocation(), diag::note_property_declare); } + Qualifiers qs; - qs.addObjCLifetime(Qualifiers::OCL_Weak); + qs.addObjCLifetime(lifetime); PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs); } - else if (kind & ObjCPropertyDecl::OBJC_PR_assign && - PropertyIvarType->isObjCRetainableType()) { - // assume that an 'assign' property synthesizes __unsafe_unretained - // ivar - Qualifiers qs; - qs.addObjCLifetime(Qualifiers::OCL_ExplicitNone); - PropertyIvarType = Context.getQualifiedType(PropertyIvarType, qs); - } } if (kind & ObjCPropertyDecl::OBJC_PR_weak && @@ -685,7 +670,7 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, QualType IvarType = Context.getCanonicalType(Ivar->getType()); // Check that type of property and its ivar are type compatible. - if (PropertyIvarType != IvarType) { + if (Context.getCanonicalType(PropertyIvarType) != IvarType) { bool compat = false; if (isa(PropertyIvarType) && isa(IvarType)) @@ -1375,10 +1360,10 @@ Sema::AtomicPropertySetterGetterRules (ObjCImplDecl* IMPDecl, bool LookedUpGetterSetter = false; unsigned Attributes = Property->getPropertyAttributes(); - unsigned AttributesAsWrittern = Property->getPropertyAttributesAsWritten(); + unsigned AttributesAsWritten = Property->getPropertyAttributesAsWritten(); - if (!(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_atomic) && - !(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_nonatomic)) { + if (!(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_atomic) && + !(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_nonatomic)) { GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName()); SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName()); LookedUpGetterSetter = true; diff --git a/test/PCH/chain-remap-types.m b/test/PCH/chain-remap-types.m index 7886332faf..585da44865 100644 --- a/test/PCH/chain-remap-types.m +++ b/test/PCH/chain-remap-types.m @@ -5,7 +5,7 @@ // CHECK: @class X; // CHECK: struct Y -// CHECK: @property ( assign,readwrite ) X * prop +// CHECK: @property ( assign,readwrite,atomic ) X * prop // CHECK: void h(X *); // CHECK: @interface X(Blah) // CHECK: void g(X *);