Author: rjmccall Date: Wed Sep 23 17:14:21 2015 New Revision: 248436 URL: http://llvm.org/viewvc/llvm-project?rev=248436&view=rev Log: Forbid qualifiers on ObjC generic parameters and arguments, but silently ignore them on arguments when they're provided indirectly (.e.g behind a template argument or typedef).
This is mostly just good language design --- specifying that a generic argument is __weak doesn't actually do anything --- but it also prevents assertions when trying to apply a different ownership qualifier. rdar://21612439 Added: cfe/trunk/test/SemaObjC/parameterized_classes_arc.m cfe/trunk/test/SemaObjCXX/parameterized_classes_arc.mm Modified: cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/AST/TypeLoc.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/AST/TypeLoc.cpp cfe/trunk/lib/Sema/SemaDeclObjC.cpp cfe/trunk/lib/Sema/SemaType.cpp Modified: cfe/trunk/include/clang/AST/Type.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Type.h (original) +++ cfe/trunk/include/clang/AST/Type.h Wed Sep 23 17:14:21 2015 @@ -3650,6 +3650,23 @@ public: bool isSugared() const { return true; } QualType desugar() const { return getEquivalentType(); } + /// Does this attribute behave like a type qualifier? + /// + /// A type qualifier adjusts a type to provide specialized rules for + /// a specific object, like the standard const and volatile qualifiers. + /// This includes attributes controlling things like nullability, + /// address spaces, and ARC ownership. The value of the object is still + /// largely described by the modified type. + /// + /// In contrast, many type attributes "rewrite" their modified type to + /// produce a fundamentally different type, not necessarily related in any + /// formalizable way to the original type. For example, calling convention + /// and vector attributes are not simple type qualifiers. + /// + /// Type qualifiers are often, but not always, reflected in the canonical + /// type. + bool isQualifier() const; + bool isMSTypeSpec() const; bool isCallingConv() const; Modified: cfe/trunk/include/clang/AST/TypeLoc.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TypeLoc.h?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/TypeLoc.h (original) +++ cfe/trunk/include/clang/AST/TypeLoc.h Wed Sep 23 17:14:21 2015 @@ -151,6 +151,14 @@ public: TypeLoc IgnoreParens() const; + /// \brief Find a type with the location of an explicit type qualifier. + /// + /// The result, if non-null, will be one of: + /// QualifiedTypeLoc + /// AtomicTypeLoc + /// AttributedTypeLoc, for those type attributes that behave as qualifiers + TypeLoc findExplicitQualifierLoc() const; + /// \brief Initializes this to state that every location in this /// type is the given location. /// @@ -737,6 +745,10 @@ public: return hasAttrExprOperand() || hasAttrEnumOperand(); } + bool isQualifier() const { + return getTypePtr()->isQualifier(); + } + /// The modified type, which is generally canonically different from /// the attribute type. /// int main(int, char**) __attribute__((noreturn)) Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 23 17:14:21 2015 @@ -7843,10 +7843,10 @@ def warn_nullability_missing : Warning< "type specifier (_Nonnull, _Nullable, or _Null_unspecified)">, InGroup<NullabilityCompleteness>; -def err_type_arg_explicit_nullability : Error< +def err_objc_type_arg_explicit_nullability : Error< "type argument %0 cannot explicitly specify nullability">; -def err_type_param_bound_explicit_nullability : Error< +def err_objc_type_param_bound_explicit_nullability : Error< "type parameter %0 bound %1 cannot explicitly specify nullability">; } @@ -7858,6 +7858,8 @@ def err_objc_type_param_bound_nonobject def err_objc_type_param_bound_missing_pointer : Error< "missing '*' in type bound %0 for type parameter %1">; +def err_objc_type_param_bound_qualified : Error< + "type bound %1 for type parameter %0 cannot be qualified with '%2'">; def err_objc_type_param_redecl : Error< "redeclaration of type parameter %0">; @@ -7892,6 +7894,8 @@ def err_objc_parameterized_forward_class def err_objc_type_arg_missing_star : Error< "type argument %0 must be a pointer (requires a '*')">; +def err_objc_type_arg_qualified : Error< + "type argument %0 cannot be qualified with '%1'">; def err_objc_type_arg_missing : Error< "no type or protocol named %0">; Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Wed Sep 23 17:14:21 2015 @@ -2944,6 +2944,47 @@ bool TagType::isBeingDefined() const { return getDecl()->isBeingDefined(); } +bool AttributedType::isQualifier() const { + switch (getAttrKind()) { + // These are type qualifiers in the traditional C sense: they annotate + // something about a specific value/variable of a type. (They aren't + // always part of the canonical type, though.) + case AttributedType::attr_address_space: + case AttributedType::attr_objc_gc: + case AttributedType::attr_objc_ownership: + case AttributedType::attr_nonnull: + case AttributedType::attr_nullable: + case AttributedType::attr_null_unspecified: + return true; + + // These aren't qualifiers; they rewrite the modified type to be a + // semantically different type. + case AttributedType::attr_regparm: + case AttributedType::attr_vector_size: + case AttributedType::attr_neon_vector_type: + case AttributedType::attr_neon_polyvector_type: + case AttributedType::attr_pcs: + case AttributedType::attr_pcs_vfp: + case AttributedType::attr_noreturn: + case AttributedType::attr_cdecl: + case AttributedType::attr_fastcall: + case AttributedType::attr_stdcall: + case AttributedType::attr_thiscall: + case AttributedType::attr_pascal: + case AttributedType::attr_vectorcall: + case AttributedType::attr_inteloclbicc: + case AttributedType::attr_ms_abi: + case AttributedType::attr_sysv_abi: + case AttributedType::attr_ptr32: + case AttributedType::attr_ptr64: + case AttributedType::attr_sptr: + case AttributedType::attr_uptr: + case AttributedType::attr_objc_kindof: + return false; + } + llvm_unreachable("bad attributed type kind"); +} + bool AttributedType::isMSTypeSpec() const { switch (getAttrKind()) { default: return false; Modified: cfe/trunk/lib/AST/TypeLoc.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypeLoc.cpp?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/lib/AST/TypeLoc.cpp (original) +++ cfe/trunk/lib/AST/TypeLoc.cpp Wed Sep 23 17:14:21 2015 @@ -376,6 +376,27 @@ SourceLocation TypeLoc::findNullabilityL return SourceLocation(); } +TypeLoc TypeLoc::findExplicitQualifierLoc() const { + // Qualified types. + if (auto qual = getAs<QualifiedTypeLoc>()) + return qual; + + TypeLoc loc = IgnoreParens(); + + // Attributed types. + if (auto attr = loc.getAs<AttributedTypeLoc>()) { + if (attr.isQualifier()) return attr; + return attr.getModifiedLoc().findExplicitQualifierLoc(); + } + + // C11 _Atomic types. + if (auto atomic = loc.getAs<AtomicTypeLoc>()) { + return atomic; + } + + return TypeLoc(); +} + void ObjCObjectTypeLoc::initializeLocal(ASTContext &Context, SourceLocation Loc) { setHasBaseTypeAsWritten(true); Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Sep 23 17:14:21 2015 @@ -638,20 +638,44 @@ DeclResult Sema::actOnObjCTypeParam(Scop typeBoundInfo = nullptr; } - // Type bounds cannot have explicit nullability. + // Type bounds cannot have qualifiers (even indirectly) or explicit + // nullability. if (typeBoundInfo) { - // Type arguments cannot explicitly specify nullability. - if (auto nullability = AttributedType::stripOuterNullability(typeBound)) { - // Look at the type location information to find the nullability - // specifier so we can zap it. - SourceLocation nullabilityLoc - = typeBoundInfo->getTypeLoc().findNullabilityLoc(); - SourceLocation diagLoc - = nullabilityLoc.isValid()? nullabilityLoc - : typeBoundInfo->getTypeLoc().getLocStart(); - Diag(diagLoc, diag::err_type_param_bound_explicit_nullability) - << paramName << typeBoundInfo->getType() - << FixItHint::CreateRemoval(nullabilityLoc); + QualType typeBound = typeBoundInfo->getType(); + TypeLoc qual = typeBoundInfo->getTypeLoc().findExplicitQualifierLoc(); + if (qual || typeBound.hasQualifiers()) { + bool diagnosed = false; + SourceRange rangeToRemove; + if (qual) { + if (auto attr = qual.getAs<AttributedTypeLoc>()) { + rangeToRemove = attr.getLocalSourceRange(); + if (attr.getTypePtr()->getImmediateNullability()) { + Diag(attr.getLocStart(), + diag::err_objc_type_param_bound_explicit_nullability) + << paramName << typeBound + << FixItHint::CreateRemoval(rangeToRemove); + diagnosed = true; + } + } + } + + if (!diagnosed) { + Diag(qual ? qual.getLocStart() + : typeBoundInfo->getTypeLoc().getLocStart(), + diag::err_objc_type_param_bound_qualified) + << paramName << typeBound << typeBound.getQualifiers().getAsString() + << FixItHint::CreateRemoval(rangeToRemove); + } + + // If the type bound has qualifiers other than CVR, we need to strip + // them or we'll probably assert later when trying to apply new + // qualifiers. + Qualifiers quals = typeBound.getQualifiers(); + quals.removeCVRQualifiers(); + if (!quals.empty()) { + typeBoundInfo = + Context.getTrivialTypeSourceInfo(typeBound.getUnqualifiedType()); + } } } } Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=248436&r1=248435&r2=248436&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Sep 23 17:14:21 2015 @@ -790,18 +790,33 @@ static QualType applyObjCTypeArgs(Sema & TypeSourceInfo *typeArgInfo = typeArgs[i]; QualType typeArg = typeArgInfo->getType(); - // Type arguments cannot explicitly specify nullability. - if (auto nullability = AttributedType::stripOuterNullability(typeArg)) { - SourceLocation nullabilityLoc - = typeArgInfo->getTypeLoc().findNullabilityLoc(); - SourceLocation diagLoc = nullabilityLoc.isValid()? nullabilityLoc - : typeArgInfo->getTypeLoc().getLocStart(); - S.Diag(diagLoc, - diag::err_type_arg_explicit_nullability) - << typeArg - << FixItHint::CreateRemoval(nullabilityLoc); + // Type arguments cannot have explicit qualifiers or nullability. + // We ignore indirect sources of these, e.g. behind typedefs or + // template arguments. + if (TypeLoc qual = typeArgInfo->getTypeLoc().findExplicitQualifierLoc()) { + bool diagnosed = false; + SourceRange rangeToRemove; + if (auto attr = qual.getAs<AttributedTypeLoc>()) { + rangeToRemove = attr.getLocalSourceRange(); + if (attr.getTypePtr()->getImmediateNullability()) { + typeArg = attr.getTypePtr()->getModifiedType(); + S.Diag(attr.getLocStart(), + diag::err_objc_type_arg_explicit_nullability) + << typeArg << FixItHint::CreateRemoval(rangeToRemove); + diagnosed = true; + } + } + + if (!diagnosed) { + S.Diag(qual.getLocStart(), diag::err_objc_type_arg_qualified) + << typeArg << typeArg.getQualifiers().getAsString() + << FixItHint::CreateRemoval(rangeToRemove); + } } + // Remove qualifiers even if they're non-local. + typeArg = typeArg.getUnqualifiedType(); + finalTypeArgs.push_back(typeArg); if (typeArg->getAs<PackExpansionType>()) Added: cfe/trunk/test/SemaObjC/parameterized_classes_arc.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/parameterized_classes_arc.m?rev=248436&view=auto ============================================================================== --- cfe/trunk/test/SemaObjC/parameterized_classes_arc.m (added) +++ cfe/trunk/test/SemaObjC/parameterized_classes_arc.m Wed Sep 23 17:14:21 2015 @@ -0,0 +1,107 @@ +// RUN: %clang_cc1 -fblocks -fobjc-arc -fobjc-runtime-has-weak %s -verify + +// rdar://21612439 + +__attribute__((objc_root_class)) +@interface NSObject +@end + +@class Forward; +@class Forward2; + +// Tests for generic arguments. + +@interface PC1<T> : NSObject +- (T) get; +- (void) set: (T) v; // expected-note 4 {{parameter}} +@end + +void test1a(PC1<__weak id> *obj) { // expected-error {{type argument '__weak id' cannot be qualified with '__weak'}} + id x = [obj get]; + [obj set: x]; +} + +void test1b(PC1<__strong id> *obj) { // expected-error {{type argument '__strong id' cannot be qualified with '__strong'}} + id x = [obj get]; + [obj set: x]; +} + +void test1c(PC1<id> *obj) { + id x = [obj get]; + [obj set: x]; +} + +// Test that this doesn't completely kill downstream type-checking. +void test1d(PC1<__weak Forward*> *obj) { // expected-error {{type argument 'Forward *__weak' cannot be qualified with '__weak'}} + Forward2 *x = [obj get]; // expected-warning {{incompatible}} + [obj set: x]; // expected-warning {{incompatible}} +} + +void test1e(PC1<__strong Forward*> *obj) { // expected-error {{type argument 'Forward *__strong' cannot be qualified with '__strong'}} + Forward2 *x = [obj get]; // expected-warning {{incompatible}} + [obj set: x]; // expected-warning {{incompatible}} +} + +void test1f(PC1<Forward*> *obj) { + Forward2 *x = [obj get]; // expected-warning {{incompatible}} + [obj set: x]; // expected-warning {{incompatible}} +} + +// Typedefs are fine, just silently ignore them. +typedef __strong id StrongID; +void test1g(PC1<StrongID> *obj) { + Forward2 *x = [obj get]; + [obj set: x]; +} + +typedef __strong Forward *StrongForward; +void test1h(PC1<StrongForward> *obj) { + Forward2 *x = [obj get]; // expected-warning {{incompatible}} + [obj set: x]; // expected-warning {{incompatible}} +} + +// These aren't really ARC-specific, but they're the same basic idea. +void test1i(PC1<const id> *obj) { // expected-error {{type argument 'const id' cannot be qualified with 'const'}} + id x = [obj get]; + [obj set: x]; +} + +void test1j(PC1<volatile id> *obj) { // expected-error {{type argument 'volatile id' cannot be qualified with 'volatile'}} + id x = [obj get]; + [obj set: x]; +} + +void test1k(PC1<__attribute__((address_space(256))) id> *obj) { // expected-error {{type argument '__attribute__((address_space(256))) id' cannot be qualified with '__attribute__((address_space(256)))'}} + id x = [obj get]; + [obj set: x]; +} + +// Tests for generic parameter bounds. + +@interface PC2<T : __strong id> // expected-error {{type bound '__strong id' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC3<T : __weak id> // expected-error {{type bound '__weak id' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC4<T : __strong Forward*> // expected-error {{type bound 'Forward *__strong' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC5<T : __weak Forward*> // expected-error {{type bound 'Forward *__weak' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC6<T : StrongID> // expected-error {{type bound 'StrongID' (aka '__strong id') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC7<T : StrongForward> // expected-error {{type bound 'StrongForward' (aka 'Forward *__strong') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +// These aren't really ARC-specific, but they're the same basic idea. +@interface PC8<T : const id> // expected-error {{type bound 'const id' for type parameter 'T' cannot be qualified with 'const'}} +@end + +@interface PC9<T : volatile id> // expected-error {{type bound 'volatile id' for type parameter 'T' cannot be qualified with 'volatile'}} +@end + +@interface PC10<T : __attribute__((address_space(256))) id> // expected-error {{type bound '__attribute__((address_space(256))) id' for type parameter 'T' cannot be qualified with '__attribute__((address_space(256)))'}} +@end \ No newline at end of file Added: cfe/trunk/test/SemaObjCXX/parameterized_classes_arc.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/parameterized_classes_arc.mm?rev=248436&view=auto ============================================================================== --- cfe/trunk/test/SemaObjCXX/parameterized_classes_arc.mm (added) +++ cfe/trunk/test/SemaObjCXX/parameterized_classes_arc.mm Wed Sep 23 17:14:21 2015 @@ -0,0 +1,119 @@ +// RUN: %clang_cc1 -fblocks -fobjc-arc -fobjc-runtime-has-weak %s -verify + +// rdar://21612439 + +__attribute__((objc_root_class)) +@interface NSObject +@end + +@class Forward; +@class Forward2; + +// Tests for generic arguments. + +@interface PC1<T> : NSObject +- (T) get; +- (void) set: (T) v; +@end + +void test1a(PC1<__weak id> *obj) { // expected-error {{type argument '__weak id' cannot be qualified with '__weak'}} + id x = [obj get]; + [obj set: x]; +} + +void test1b(PC1<__strong id> *obj) { // expected-error {{type argument '__strong id' cannot be qualified with '__strong'}} + id x = [obj get]; + [obj set: x]; +} + +void test1c(PC1<id> *obj) { + id x = [obj get]; + [obj set: x]; +} + +// Test that this doesn't completely kill downstream type-checking. +void test1d(PC1<__weak Forward*> *obj) { // expected-error {{type argument 'Forward *__weak' cannot be qualified with '__weak'}} + Forward2 *x = [obj get]; // expected-error {{cannot initialize}} + [obj set: x]; +} + +void test1e(PC1<__strong Forward*> *obj) { // expected-error {{type argument 'Forward *__strong' cannot be qualified with '__strong'}} + Forward2 *x = [obj get]; // expected-error {{cannot initialize}} + [obj set: x]; +} + +void test1f(PC1<Forward*> *obj) { + Forward2 *x = [obj get]; // expected-error {{cannot initialize}} + [obj set: x]; +} + +// Typedefs are fine, just silently ignore them. +typedef __strong id StrongID; +void test1g(PC1<StrongID> *obj) { + Forward2 *x = [obj get]; + [obj set: x]; +} + +typedef __strong Forward *StrongForward; +void test1h(PC1<StrongForward> *obj) { + Forward2 *x = [obj get]; // expected-error {{cannot initialize}} + [obj set: x]; +} + +// These aren't really ARC-specific, but they're the same basic idea. +void test1i(PC1<const id> *obj) { // expected-error {{type argument 'const id' cannot be qualified with 'const'}} + id x = [obj get]; + [obj set: x]; +} + +void test1j(PC1<volatile id> *obj) { // expected-error {{type argument 'volatile id' cannot be qualified with 'volatile'}} + id x = [obj get]; + [obj set: x]; +} + +void test1k(PC1<__attribute__((address_space(256))) id> *obj) { // expected-error {{type argument '__attribute__((address_space(256))) id' cannot be qualified with '__attribute__((address_space(256)))'}} + id x = [obj get]; + [obj set: x]; +} + +// Template-specific tests. +template <class T> PC1<T> *test2_temp(); +void test2a() { test2_temp<id>(); } +void test2b() { test2_temp<const id>(); } +void test2c() { test2_temp<volatile id>(); } +void test2d() { test2_temp<__strong id>(); } +void test2e() { test2_temp<__weak id>(); } +void test2f() { test2_temp<__attribute__((address_space(256))) id>(); } + +template <class T> PC1<const T> *test3a(); // expected-error {{type argument 'const T' cannot be qualified with 'const'}} +template <class T> PC1<__strong T> *test3b(); // expected-error {{type argument '__strong T' cannot be qualified with '__strong'}} + +// Tests for generic parameter bounds. + +@interface PC2<T : __strong id> // expected-error {{type bound '__strong id' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC3<T : __weak id> // expected-error {{type bound '__weak id' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC4<T : __strong Forward*> // expected-error {{type bound 'Forward *__strong' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC5<T : __weak Forward*> // expected-error {{type bound 'Forward *__weak' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC6<T : StrongID> // expected-error {{type bound 'StrongID' (aka '__strong id') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC7<T : StrongForward> // expected-error {{type bound 'StrongForward' (aka 'Forward *__strong') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +// These aren't really ARC-specific, but they're the same basic idea. +@interface PC8<T : const id> // expected-error {{type bound 'const id' for type parameter 'T' cannot be qualified with 'const'}} +@end + +@interface PC9<T : volatile id> // expected-error {{type bound 'volatile id' for type parameter 'T' cannot be qualified with 'volatile'}} +@end + +@interface PC10<T : __attribute__((address_space(256))) id> // expected-error {{type bound '__attribute__((address_space(256))) id' for type parameter 'T' cannot be qualified with '__attribute__((address_space(256)))'}} +@end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits