From bb98aee681b328c134d60f31a4f68417877352f6 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 23 Sep 2015 22:14:21 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248436 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Type.h | 17 +++ include/clang/AST/TypeLoc.h | 12 ++ include/clang/Basic/DiagnosticSemaKinds.td | 8 +- lib/AST/Type.cpp | 41 +++++++ lib/AST/TypeLoc.cpp | 21 ++++ lib/Sema/SemaDeclObjC.cpp | 50 ++++++-- lib/Sema/SemaType.cpp | 35 ++++-- test/SemaObjC/parameterized_classes_arc.m | 107 +++++++++++++++++ test/SemaObjCXX/parameterized_classes_arc.mm | 119 +++++++++++++++++++ 9 files changed, 385 insertions(+), 25 deletions(-) create mode 100644 test/SemaObjC/parameterized_classes_arc.m create mode 100644 test/SemaObjCXX/parameterized_classes_arc.mm diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index a44b3120f7..075fe87dab 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -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; diff --git a/include/clang/AST/TypeLoc.h b/include/clang/AST/TypeLoc.h index df68845d7c..674fffb8fa 100644 --- a/include/clang/AST/TypeLoc.h +++ b/include/clang/AST/TypeLoc.h @@ -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)) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 5b03968609..fbd3665476 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7843,10 +7843,10 @@ def warn_nullability_missing : Warning< "type specifier (_Nonnull, _Nullable, or _Null_unspecified)">, InGroup; -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 : Error< 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_first : Error< 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">; diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 44e8406f2f..40fe903fea 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -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; diff --git a/lib/AST/TypeLoc.cpp b/lib/AST/TypeLoc.cpp index a057a345a9..f7f7b05b38 100644 --- a/lib/AST/TypeLoc.cpp +++ b/lib/AST/TypeLoc.cpp @@ -376,6 +376,27 @@ SourceLocation TypeLoc::findNullabilityLoc() const { return SourceLocation(); } +TypeLoc TypeLoc::findExplicitQualifierLoc() const { + // Qualified types. + if (auto qual = getAs()) + return qual; + + TypeLoc loc = IgnoreParens(); + + // Attributed types. + if (auto attr = loc.getAs()) { + if (attr.isQualifier()) return attr; + return attr.getModifiedLoc().findExplicitQualifierLoc(); + } + + // C11 _Atomic types. + if (auto atomic = loc.getAs()) { + return atomic; + } + + return TypeLoc(); +} + void ObjCObjectTypeLoc::initializeLocal(ASTContext &Context, SourceLocation Loc) { setHasBaseTypeAsWritten(true); diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index f42c4b7546..7b9b9c148c 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -638,20 +638,44 @@ DeclResult Sema::actOnObjCTypeParam(Scope *S, 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()) { + 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()); + } } } } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 73da718a81..96c51a94eb 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -790,18 +790,33 @@ static QualType applyObjCTypeArgs(Sema &S, SourceLocation loc, QualType type, 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()) { + 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()) diff --git a/test/SemaObjC/parameterized_classes_arc.m b/test/SemaObjC/parameterized_classes_arc.m new file mode 100644 index 0000000000..608a521f47 --- /dev/null +++ b/test/SemaObjC/parameterized_classes_arc.m @@ -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 : 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 *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 *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 *obj) { + Forward2 *x = [obj get]; + [obj set: x]; +} + +typedef __strong Forward *StrongForward; +void test1h(PC1 *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 *obj) { // expected-error {{type argument 'const id' cannot be qualified with 'const'}} + id x = [obj get]; + [obj set: x]; +} + +void test1j(PC1 *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 // expected-error {{type bound '__strong id' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC3 // expected-error {{type bound '__weak id' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC4 // expected-error {{type bound 'Forward *__strong' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC5 // expected-error {{type bound 'Forward *__weak' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC6 // expected-error {{type bound 'StrongID' (aka '__strong id') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC7 // 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 // expected-error {{type bound 'const id' for type parameter 'T' cannot be qualified with 'const'}} +@end + +@interface PC9 // expected-error {{type bound 'volatile id' for type parameter 'T' cannot be qualified with 'volatile'}} +@end + +@interface PC10 // 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 diff --git a/test/SemaObjCXX/parameterized_classes_arc.mm b/test/SemaObjCXX/parameterized_classes_arc.mm new file mode 100644 index 0000000000..38ddd70986 --- /dev/null +++ b/test/SemaObjCXX/parameterized_classes_arc.mm @@ -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 : 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 *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 *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 *obj) { + Forward2 *x = [obj get]; + [obj set: x]; +} + +typedef __strong Forward *StrongForward; +void test1h(PC1 *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 *obj) { // expected-error {{type argument 'const id' cannot be qualified with 'const'}} + id x = [obj get]; + [obj set: x]; +} + +void test1j(PC1 *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 PC1 *test2_temp(); +void test2a() { test2_temp(); } +void test2b() { test2_temp(); } +void test2c() { test2_temp(); } +void test2d() { test2_temp<__strong id>(); } +void test2e() { test2_temp<__weak id>(); } +void test2f() { test2_temp<__attribute__((address_space(256))) id>(); } + +template PC1 *test3a(); // expected-error {{type argument 'const T' cannot be qualified with 'const'}} +template PC1<__strong T> *test3b(); // expected-error {{type argument '__strong T' cannot be qualified with '__strong'}} + +// Tests for generic parameter bounds. + +@interface PC2 // expected-error {{type bound '__strong id' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC3 // expected-error {{type bound '__weak id' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC4 // expected-error {{type bound 'Forward *__strong' for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC5 // expected-error {{type bound 'Forward *__weak' for type parameter 'T' cannot be qualified with '__weak'}} +@end + +@interface PC6 // expected-error {{type bound 'StrongID' (aka '__strong id') for type parameter 'T' cannot be qualified with '__strong'}} +@end + +@interface PC7 // 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 // expected-error {{type bound 'const id' for type parameter 'T' cannot be qualified with 'const'}} +@end + +@interface PC9 // expected-error {{type bound 'volatile id' for type parameter 'T' cannot be qualified with 'volatile'}} +@end + +@interface PC10 // expected-error {{type bound '__attribute__((address_space(256))) id' for type parameter 'T' cannot be qualified with '__attribute__((address_space(256)))'}} +@end -- 2.40.0