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
diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index 67ee937..daf0b68 100644
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -3236,6 +3236,36 @@
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<AttributedType>(type)) {
+ if (attr->getAttrKind() == AttributedType::attr_objc_ownership)
+ return true;
+
+ type = attr->getModifiedType();
+
+ // X *__strong (...)
+ } else if (const ParenType *paren = dyn_cast<ParenType>(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 @@
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 @@
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 @@
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 @@
<< 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 82c6389..ce525db 100644
--- a/test/SemaObjC/arc-objc-lifetime.m
+++ b/test/SemaObjC/arc-objc-lifetime.m
@@ -32,3 +32,11 @@
__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 7a96550..d9ae5b4 100644
--- a/test/SemaObjC/arc.m
+++ b/test/SemaObjC/arc.m
@@ -39,8 +39,6 @@
}
@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 931b21f..9eca846 100644
--- a/test/SemaObjCXX/arc-templates.mm
+++ b/test/SemaObjCXX/arc-templates.mm
@@ -97,8 +97,8 @@
template<typename T>
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<make_weak_fail<__weak id>::type, __weak id>::value? 1 : -1]; // expected-note{{in instantiation of template class 'make_weak_fail<__weak id>' requested here}}