From: Douglas Gregor Date: Fri, 17 Jun 2011 23:16:24 +0000 (+0000) Subject: Objective-C++ ARC: eliminate the utterly unjustified loophole that X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4020caec546d221170072d2388b57d151cb26111;p=clang Objective-C++ ARC: eliminate the utterly unjustified loophole that silently dropped ownership qualifiers that were being applied to ownership-qualified, substituted type that was *not* a substituted template type parameter. We now provide a diagnostic in such cases, and recover by dropping the added qualifiers. Document this behavior in the ARC specification. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@133309 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/docs/AutomaticReferenceCounting.html b/docs/AutomaticReferenceCounting.html index b5b17b7273..d094d74125 100644 --- a/docs/AutomaticReferenceCounting.html +++ b/docs/AutomaticReferenceCounting.html @@ -598,12 +598,15 @@ if it is a retainable object pointer type or an array type whose element type is a retainable object owner type.

An ownership qualifier is a type -qualifier which applies only to retainable object owner types. A -program is ill-formed if it attempts to apply an ownership qualifier +qualifier which applies only to retainable object owner types. An array type is +ownership-qualified according to its element type, and adding an ownership +qualifier to an array type so qualifies its element type.

+ +

A program is ill-formed if it attempts to apply an ownership qualifier to a type which is already ownership-qualified, even if it is the same -qualifier. An array type is ownership-qualified according to its -element type, and adding an ownership qualifier to an array type so -qualifies its element type.

+qualifier. There is a single exception to this rule: an ownership qualifier +may be applied to a substituted template type parameter, which overrides the +ownership qualifier provided by the template argument.

Except as described under the inference rules, a program is @@ -612,7 +615,7 @@ retainable object owner type which lacks an ownership qualifier.

Rationale: these rules, together with the inference rules, ensure that all objects and lvalues of retainable -object pointer type have an ownership qualifier.

+object pointer type have an ownership qualifier. The ability to override an ownership qualifier during template substitution is required to counteract the inference of __strong for template type arguments.

There are four ownership qualifiers:

diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index d33e7a5c4c..121ff00628 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -3177,8 +3177,7 @@ TreeTransform::TransformQualifiedType(TypeLocBuilder &TLB, if (Quals.hasObjCLifetime()) { if (!Result->isObjCLifetimeType() && !Result->isDependentType()) Quals.removeObjCLifetime(); - else if (Result.getObjCLifetime() && - Result.getObjCLifetime() != Quals.getObjCLifetime()) { + else if (Result.getObjCLifetime()) { // Objective-C ARC: // A lifetime qualifier applied to a substituted template parameter // overrides the lifetime qualifier from the template argument. @@ -3195,8 +3194,12 @@ TreeTransform::TransformQualifiedType(TypeLocBuilder &TLB, Replacement); TLB.TypeWasModifiedSafely(Result); } else { - // Otherwise, drop the new qualifier. - // FIXME: I don't recall the justification for this! + // Otherwise, complain about the addition of a qualifier to an + // already-qualified type. + SourceRange R = TLB.getTemporaryTypeLoc(Result).getSourceRange(); + SemaRef.Diag(R.getBegin(), diag::err_attr_objc_lifetime_redundant) + << Result << R; + Quals.removeObjCLifetime(); } } diff --git a/lib/Sema/TypeLocBuilder.h b/lib/Sema/TypeLocBuilder.h index 792bd1fc72..f7889e9c64 100644 --- a/lib/Sema/TypeLocBuilder.h +++ b/lib/Sema/TypeLocBuilder.h @@ -147,7 +147,7 @@ private: Index -= LocalSize; - return getTypeLoc(T); + return getTemporaryTypeLoc(T); } /// Grow to the given capacity. @@ -179,15 +179,17 @@ private: reserve(Size); Index -= Size; - return getTypeLoc(T); + return getTemporaryTypeLoc(T); } - - // This is private because, when we kill off TypeSourceInfo in favor - // of TypeLoc, we'll want an interface that creates a TypeLoc given - // an ASTContext, and we don't want people to think they can just - // use this as an equivalent. - TypeLoc getTypeLoc(QualType T) { +public: + /// \brief Retrieve a temporary TypeLoc that refers into this \c TypeLocBuilder + /// object. + /// + /// The resulting \c TypeLoc should only be used so long as the + /// \c TypeLocBuilder is active and has not had more type information + /// pushed into it. + TypeLoc getTemporaryTypeLoc(QualType T) { #ifndef NDEBUG assert(LastTy == T && "type doesn't match last type pushed!"); #endif diff --git a/test/SemaObjCXX/arc-templates.mm b/test/SemaObjCXX/arc-templates.mm index 3711bd72df..73fc314e0e 100644 --- a/test/SemaObjCXX/arc-templates.mm +++ b/test/SemaObjCXX/arc-templates.mm @@ -94,6 +94,16 @@ int check_make_weak0[is_same::type, __weak id>::value? 1 : -1]; int check_make_weak1[is_same::type, __weak id>::value? 1 : -1]; int check_make_weak2[is_same::type, __weak id>::value? 1 : -1]; +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}} +}; + +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}} + +int check_make_weak_fail1[is_same::type, __weak id>::value? -1 : 1]; // expected-note{{in instantiation of template class 'make_weak_fail' requested here}} // Check template argument deduction from function templates. template struct identity { };