[cfe-commits] r139615 - in /cfe/trunk: include/clang/AST/DeclObjC.h include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/CGObjC.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaObjCProperty.cpp test/PCH/chain-remap-types.m
John McCall
rjmccall at apple.com
Tue Sep 13 11:31:23 PDT 2011
Author: rjmccall
Date: Tue Sep 13 13:31:23 2011
New Revision: 139615
URL: http://llvm.org/viewvc/llvm-project?rev=139615&view=rev
Log:
Refactoring, mostly to give ObjCPropertyDecls stronger invariants for
their semantic attributes and then to take advantage of that.
Modified:
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaObjCProperty.cpp
cfe/trunk/test/PCH/chain-remap-types.m
Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Tue Sep 13 13:31:23 2011
@@ -1495,6 +1495,17 @@
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.
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 13 13:31:23 2011
@@ -2886,7 +2886,7 @@
"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 "
Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Tue Sep 13 13:31:23 2011
@@ -443,10 +443,10 @@
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 @@
= 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 @@
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.
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 13 13:31:23 2011
@@ -3862,10 +3862,7 @@
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))
Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Tue Sep 13 13:31:23 2011
@@ -24,6 +24,37 @@
// 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 @@
// 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 @@
S.Diag(property->getLocation(),
diag::err_arc_inconsistent_property_ownership)
<< property->getDeclName()
- << selector
+ << expectedLifetime
<< propertyLifetime;
}
@@ -393,9 +421,10 @@
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 @@
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;
- }
- // 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;
- }
+ // The lifetime implied by the property's attributes.
+ Qualifiers::ObjCLifetime propertyLifetime =
+ getImpliedARCOwnership(property->getPropertyAttributes(),
+ property->getType());
+
+ // We're fine if they match.
+ if (propertyLifetime == ivarLifetime) return;
+
+ // These aren't valid lifetimes for object ivars; don't diagnose twice.
+ if (ivarLifetime == Qualifiers::OCL_None ||
+ ivarLifetime == Qualifiers::OCL_Autoreleasing)
+ return;
- // Any other property should be ignored.
- } else {
+ 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 @@
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<ReferenceType>(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 @@
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<ObjCObjectPointerType>(PropertyIvarType)
&& isa<ObjCObjectPointerType>(IvarType))
@@ -1375,10 +1360,10 @@
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;
Modified: cfe/trunk/test/PCH/chain-remap-types.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/chain-remap-types.m?rev=139615&r1=139614&r2=139615&view=diff
==============================================================================
--- cfe/trunk/test/PCH/chain-remap-types.m (original)
+++ cfe/trunk/test/PCH/chain-remap-types.m Tue Sep 13 13:31:23 2011
@@ -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 *);
More information about the cfe-commits
mailing list