From d85bf9dd23920f5400bb8d6a41c8ebb1aecfdee9 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 8 Feb 2012 00:46:41 +0000 Subject: [PATCH] Only complain about __strong __strong id, not __strong SomeStrongTypedef or __strong __typeof__(some.strong.thing). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150029 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaType.cpp | 74 +++++++++++++++++++++++++------ test/SemaObjC/arc-objc-lifetime.m | 8 ++++ test/SemaObjC/arc.m | 2 - test/SemaObjCXX/arc-templates.mm | 4 +- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 67ee9376e8..daf0b683b6 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -3236,6 +3236,36 @@ static void HandleAddressSpaceTypeAttribute(QualType &Type, Type = S.Context.getAddrSpaceQualType(Type, ASIdx); } +/// Does this type have a "direct" ownership qualifier? That is, +/// is it written like "__strong id", as opposed to something like +/// "typeof(foo)", where that happens to be strong? +static bool hasDirectOwnershipQualifier(QualType type) { + // Fast path: no qualifier at all. + assert(type.getQualifiers().hasObjCLifetime()); + + while (true) { + // __strong id + if (const AttributedType *attr = dyn_cast(type)) { + if (attr->getAttrKind() == AttributedType::attr_objc_ownership) + return true; + + type = attr->getModifiedType(); + + // X *__strong (...) + } else if (const ParenType *paren = dyn_cast(type)) { + type = paren->getInnerType(); + + // That's it for things we want to complain about. In particular, + // we do not want to look through typedefs, typeof(expr), + // typeof(type), or any other way that the type is somehow + // abstracted. + } else { + + return false; + } + } +} + /// handleObjCOwnershipTypeAttr - Process an objc_ownership /// attribute on the specified type. /// @@ -3264,12 +3294,6 @@ static bool handleObjCOwnershipTypeAttr(TypeProcessingState &state, if (AttrLoc.isMacroID()) AttrLoc = S.getSourceManager().getImmediateExpansionRange(AttrLoc).first; - if (type.getQualifiers().getObjCLifetime()) { - S.Diag(AttrLoc, diag::err_attr_objc_ownership_redundant) - << type; - return true; - } - if (!attr.getParameterName()) { S.Diag(AttrLoc, diag::err_attribute_argument_n_not_string) << "objc_ownership" << 1; @@ -3277,6 +3301,11 @@ static bool handleObjCOwnershipTypeAttr(TypeProcessingState &state, return true; } + // Consume lifetime attributes without further comment outside of + // ARC mode. + if (!S.getLangOptions().ObjCAutoRefCount) + return true; + Qualifiers::ObjCLifetime lifetime; if (attr.getParameterName()->isStr("none")) lifetime = Qualifiers::OCL_ExplicitNone; @@ -3293,10 +3322,31 @@ static bool handleObjCOwnershipTypeAttr(TypeProcessingState &state, return true; } - // Consume lifetime attributes without further comment outside of - // ARC mode. - if (!S.getLangOptions().ObjCAutoRefCount) - return true; + SplitQualType underlyingType = type.split(); + + // Check for redundant/conflicting ownership qualifiers. + if (Qualifiers::ObjCLifetime previousLifetime + = type.getQualifiers().getObjCLifetime()) { + // If it's written directly, that's an error. + if (hasDirectOwnershipQualifier(type)) { + S.Diag(AttrLoc, diag::err_attr_objc_ownership_redundant) + << type; + return true; + } + + // Otherwise, if the qualifiers actually conflict, pull sugar off + // until we reach a type that is directly qualified. + if (previousLifetime != lifetime) { + // This should always terminate: the canonical type is + // qualified, so some bit of sugar must be hiding it. + while (!underlyingType.Quals.hasObjCLifetime()) { + underlyingType = underlyingType.getSingleStepDesugaredType(); + } + underlyingType.Quals.removeObjCLifetime(); + } + } + + underlyingType.Quals.addObjCLifetime(lifetime); if (NonObjCPointer) { StringRef name = attr.getName()->getName(); @@ -3312,11 +3362,9 @@ static bool handleObjCOwnershipTypeAttr(TypeProcessingState &state, << name << type; } - Qualifiers qs; - qs.setObjCLifetime(lifetime); QualType origType = type; if (!NonObjCPointer) - type = S.Context.getQualifiedType(type, qs); + type = S.Context.getQualifiedType(underlyingType); // If we have a valid source location for the attribute, use an // AttributedType instead. diff --git a/test/SemaObjC/arc-objc-lifetime.m b/test/SemaObjC/arc-objc-lifetime.m index 82c6389271..ce525db7b5 100644 --- a/test/SemaObjC/arc-objc-lifetime.m +++ b/test/SemaObjC/arc-objc-lifetime.m @@ -32,3 +32,11 @@ typedef __autoreleasing NSString * AUTORELEASEPNSString; __autoreleasing id *stuff = (__autoreleasing id *)addr; } @end + +// rdar://problem/10711456 +__strong I *__strong test1; // expected-error {{the type 'I *__strong' is already explicitly ownership-qualified}} +__strong I *(__strong test2); // expected-error {{the type 'I *__strong' is already explicitly ownership-qualified}} +__strong I *(__strong (test3)); // expected-error {{the type 'I *__strong' is already explicitly ownership-qualified}} +__unsafe_unretained __typeof__(test3) test4; +typedef __strong I *strong_I; +__unsafe_unretained strong_I test5; diff --git a/test/SemaObjC/arc.m b/test/SemaObjC/arc.m index 7a9655037e..d9ae5b403c 100644 --- a/test/SemaObjC/arc.m +++ b/test/SemaObjC/arc.m @@ -39,8 +39,6 @@ void test1(A *a) { } @end -__weak __strong id x; // expected-error {{the type '__strong id' already has retainment attributes}} - // rdar://8843638 @interface I diff --git a/test/SemaObjCXX/arc-templates.mm b/test/SemaObjCXX/arc-templates.mm index 931b21f5d4..9eca84648f 100644 --- a/test/SemaObjCXX/arc-templates.mm +++ b/test/SemaObjCXX/arc-templates.mm @@ -97,8 +97,8 @@ int check_make_weak2[is_same::type, __weak id>::va template struct make_weak_fail { typedef T T_type; - typedef __weak T_type type; // expected-error{{the type 'T_type' (aka '__weak id') already has retainment attributes set on it}} \ - // expected-error{{the type 'T_type' (aka '__strong id') already has retainment attributes set on it}} + typedef __weak T_type type; // expected-error{{the type 'T_type' (aka '__weak id') is already explicitly ownership-qualified}} \ + // expected-error{{the type 'T_type' (aka '__strong id') is already explicitly ownership-qualified}} }; int check_make_weak_fail0[is_same::type, __weak id>::value? 1 : -1]; // expected-note{{in instantiation of template class 'make_weak_fail<__weak id>' requested here}}