From 849ebc269fe17def169c812f929746cb98955664 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Fri, 19 Jun 2015 18:14:46 +0000 Subject: [PATCH] Implement the 'null_resettable' attribute for Objective-C properties. 'null_resettable' properties are those whose getters return nonnull but whose setters take nil, to "reset" the property to some default. Implements rdar://problem/19051334. llvm-svn: 240155 --- clang/include/clang/AST/DeclObjC.h | 5 +- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++ clang/include/clang/Sema/DeclSpec.h | 5 +- clang/include/clang/Sema/Sema.h | 3 ++ clang/lib/AST/DeclPrinter.cpp | 10 +++- clang/lib/Parse/ParseObjc.cpp | 11 +++++ clang/lib/Sema/SemaDeclObjC.cpp | 3 ++ clang/lib/Sema/SemaObjCProperty.cpp | 62 +++++++++++++++++++++++- clang/test/SemaObjC/nullability.m | 39 +++++++++++++++ 9 files changed, 134 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h index 86f1af1..c3fb577 100644 --- a/clang/include/clang/AST/DeclObjC.h +++ b/clang/include/clang/AST/DeclObjC.h @@ -2206,13 +2206,14 @@ public: OBJC_PR_unsafe_unretained = 0x800, /// Indicates that the nullability of the type was spelled with a /// property attribute rather than a type qualifier. - OBJC_PR_nullability = 0x1000 + OBJC_PR_nullability = 0x1000, + OBJC_PR_null_resettable = 0x2000 // Adding a property should change NumPropertyAttrsBits }; enum { /// \brief Number of bits fitting all the property attributes. - NumPropertyAttrsBits = 13 + NumPropertyAttrsBits = 14 }; enum SetterKind { Assign, Retain, Copy, Weak }; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index cc1a80e..c86342b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7714,6 +7714,10 @@ def note_nullability_type_specifier : Note< "'%select{__nonnull|__nullable|__null_unspecified}0' to affect the innermost " "pointer type of %1">; +def warn_null_resettable_setter : Warning< + "synthesized setter %0 for null_resettable property %1 does not handle nil">, + InGroup; + } } // end of sema component. diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 9fe0cc7..7433201 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -805,7 +805,8 @@ public: DQ_PR_weak = 0x200, DQ_PR_strong = 0x400, DQ_PR_unsafe_unretained = 0x800, - DQ_PR_nullability = 0x1000 + DQ_PR_nullability = 0x1000, + DQ_PR_null_resettable = 0x2000 }; ObjCDeclSpec() @@ -865,7 +866,7 @@ private: ObjCDeclQualifier objcDeclQualifier : 7; // NOTE: VC++ treats enums as signed, avoid using ObjCPropertyAttributeKind - unsigned PropertyAttributes : 13; + unsigned PropertyAttributes : 14; unsigned Nullability : 2; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 11933ec..981f027 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2918,6 +2918,9 @@ public: ObjCContainerDecl *CDecl, bool SynthesizeProperties); + /// Diagnose any null-resettable synthesized setters. + void diagnoseNullResettableSynthesizedSetters(ObjCImplDecl *impDecl); + /// DefaultSynthesizeProperties - This routine default synthesizes all /// properties which must be synthesized in the class's \@implementation. void DefaultSynthesizeProperties (Scope *S, ObjCImplDecl* IMPDecl, diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 609cc08..d6c03169 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -1213,8 +1213,14 @@ void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) { if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_nullability) { if (auto nullability = stripOuterNullability(T)) { - Out << (first ? ' ' : ',') - << getNullabilitySpelling(*nullability).substr(2); + if (*nullability == NullabilityKind::Unspecified && + (PDecl->getPropertyAttributes() & + ObjCPropertyDecl::OBJC_PR_null_resettable)) { + Out << (first ? ' ' : ',') << "null_resettable"; + } else { + Out << (first ? ' ' : ',') + << getNullabilitySpelling(*nullability).substr(2); + } first = false; } } diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index e0f716e..8323660 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -611,6 +611,7 @@ static void diagnoseRedundantPropertyNullability(Parser &P, /// nonnull /// nullable /// null_unspecified +/// null_resettable /// void Parser::ParseObjCPropertyAttribute(ObjCDeclSpec &DS) { assert(Tok.getKind() == tok::l_paren); @@ -717,6 +718,16 @@ void Parser::ParseObjCPropertyAttribute(ObjCDeclSpec &DS) { Tok.getLocation()); DS.setPropertyAttributes(ObjCDeclSpec::DQ_PR_nullability); DS.setNullability(Tok.getLocation(), NullabilityKind::Unspecified); + } else if (II->isStr("null_resettable")) { + if (DS.getPropertyAttributes() & ObjCDeclSpec::DQ_PR_nullability) + diagnoseRedundantPropertyNullability(*this, DS, + NullabilityKind::Unspecified, + Tok.getLocation()); + DS.setPropertyAttributes(ObjCDeclSpec::DQ_PR_nullability); + DS.setNullability(Tok.getLocation(), NullabilityKind::Unspecified); + + // Also set the null_resettable bit. + DS.setPropertyAttributes(ObjCDeclSpec::DQ_PR_null_resettable); } else { Diag(AttrName, diag::err_objc_expected_property_attr) << II; SkipUntil(tok::r_paren, StopAtSemi); diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp index 78c76ab..62ba7d5 100644 --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -2024,6 +2024,9 @@ void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl, DiagnoseUnimplementedProperties(S, IMPDecl, CDecl, SynthesizeProperties); } + // Diagnose null-resettable synthesized setters. + diagnoseNullResettableSynthesizedSetters(IMPDecl); + SelectorSet ClsMap; for (const auto *I : IMPDecl->class_methods()) ClsMap.insert(I->getSelector()); diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp index f62af8e..d8e7f64 100644 --- a/clang/lib/Sema/SemaObjCProperty.cpp +++ b/clang/lib/Sema/SemaObjCProperty.cpp @@ -361,6 +361,9 @@ Sema::HandlePropertyInClassExtension(Scope *S, PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_atomic); if (Attributes & ObjCDeclSpec::DQ_PR_nullability) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_nullability); + if (Attributes & ObjCDeclSpec::DQ_PR_null_resettable) + PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_null_resettable); + // Set setter/getter selector name. Needed later. PDecl->setGetterName(GetterSel); PDecl->setSetterName(SetterSel); @@ -646,6 +649,9 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, if (Attributes & ObjCDeclSpec::DQ_PR_nullability) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_nullability); + if (Attributes & ObjCDeclSpec::DQ_PR_null_resettable) + PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_null_resettable); + return PDecl; } @@ -1760,6 +1766,33 @@ void Sema::DiagnoseUnimplementedProperties(Scope *S, ObjCImplDecl* IMPDecl, } } +void Sema::diagnoseNullResettableSynthesizedSetters(ObjCImplDecl *impDecl) { + for (const auto *propertyImpl : impDecl->property_impls()) { + const auto *property = propertyImpl->getPropertyDecl(); + + // Warn about null_resettable properties with synthesized setters, + // because the setter won't properly handle nil. + if (propertyImpl->getPropertyImplementation() + == ObjCPropertyImplDecl::Synthesize && + (property->getPropertyAttributes() & + ObjCPropertyDecl::OBJC_PR_null_resettable) && + property->getGetterMethodDecl() && + property->getSetterMethodDecl()) { + auto *getterMethod = property->getGetterMethodDecl(); + auto *setterMethod = property->getSetterMethodDecl(); + if (!impDecl->getInstanceMethod(setterMethod->getSelector()) && + !impDecl->getInstanceMethod(getterMethod->getSelector())) { + SourceLocation loc = propertyImpl->getLocation(); + if (loc.isInvalid()) + loc = impDecl->getLocStart(); + + Diag(loc, diag::warn_null_resettable_setter) + << setterMethod->getSelector() << property->getDeclName(); + } + } + } +} + void Sema::AtomicPropertySetterGetterRules (ObjCImplDecl* IMPDecl, ObjCContainerDecl* IDecl) { @@ -1995,9 +2028,21 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl *property, redeclaredProperty->getLocation() : property->getLocation(); + // If the property is null_resettable, the getter returns nonnull. + QualType resultTy = property->getType(); + if (property->getPropertyAttributes() & + ObjCPropertyDecl::OBJC_PR_null_resettable) { + QualType modifiedTy = resultTy; + if (auto nullability = AttributedType::stripOuterNullability(modifiedTy)){ + if (*nullability == NullabilityKind::Unspecified) + resultTy = Context.getAttributedType(AttributedType::attr_nonnull, + modifiedTy, modifiedTy); + } + } + GetterMethod = ObjCMethodDecl::Create(Context, Loc, Loc, property->getGetterName(), - property->getType(), nullptr, CD, + resultTy, nullptr, CD, /*isInstance=*/true, /*isVariadic=*/false, /*isPropertyAccessor=*/true, /*isImplicitlyDeclared=*/true, /*isDefined=*/false, @@ -2058,12 +2103,25 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl *property, ObjCMethodDecl::Optional : ObjCMethodDecl::Required); + // If the property is null_resettable, the setter accepts a + // nullable value. + QualType paramTy = property->getType().getUnqualifiedType(); + if (property->getPropertyAttributes() & + ObjCPropertyDecl::OBJC_PR_null_resettable) { + QualType modifiedTy = paramTy; + if (auto nullability = AttributedType::stripOuterNullability(modifiedTy)){ + if (*nullability == NullabilityKind::Unspecified) + paramTy = Context.getAttributedType(AttributedType::attr_nullable, + modifiedTy, modifiedTy); + } + } + // Invent the arguments for the setter. We don't bother making a // nice name for the argument. ParmVarDecl *Argument = ParmVarDecl::Create(Context, SetterMethod, Loc, Loc, property->getIdentifier(), - property->getType().getUnqualifiedType(), + paramTy, /*TInfo=*/nullptr, SC_None, nullptr); diff --git a/clang/test/SemaObjC/nullability.m b/clang/test/SemaObjC/nullability.m index b852584..ca875b5 100644 --- a/clang/test/SemaObjC/nullability.m +++ b/clang/test/SemaObjC/nullability.m @@ -164,6 +164,45 @@ void test_instancetype(InitializableClass * __nonnull ic, id __nonnull object) { ip = [InitializableClass returnInstanceOfMe]; // expected-warning{{incompatible pointer types assigning to 'int *' from 'InitializableClass * __nullable'}} ip = [object returnMe]; // expected-warning{{incompatible pointer types assigning to 'int *' from 'id __nullable'}} } + +// Check null_resettable getters/setters. +__attribute__((objc_root_class)) +@interface NSResettable +@property(null_resettable,retain) NSResettable *resettable1; // expected-note{{passing argument to parameter 'resettable1' here}} +@property(null_resettable,retain,nonatomic) NSResettable *resettable2; +@property(null_resettable,retain,nonatomic) NSResettable *resettable3; +@property(null_resettable,retain,nonatomic) NSResettable *resettable4; +@property(null_resettable,retain,nonatomic) NSResettable *resettable5; +@property(null_resettable,retain,nonatomic) NSResettable *resettable6; +@end + +void test_null_resettable(NSResettable *r, int *ip) { + [r setResettable1:ip]; // expected-warning{{incompatible pointer types sending 'int *' to parameter of type 'NSResettable * __nullable'}} + r.resettable1 = ip; // expected-warning{{incompatible pointer types assigning to 'NSResettable * __nullable' from 'int *'}} +} + +@implementation NSResettable // expected-warning{{synthesized setter 'setResettable4:' for null_resettable property 'resettable4' does not handle nil}} +- (NSResettable *)resettable1 { + int *ip = 0; + return ip; // expected-warning{{result type 'NSResettable * __nonnull'}} +} + +- (void)setResettable1:(NSResettable *)param { +} + +@synthesize resettable2; // no warning; not synthesized +@synthesize resettable3; // expected-warning{{synthesized setter 'setResettable3:' for null_resettable property 'resettable3' does not handle nil}} + +- (void)setResettable2:(NSResettable *)param { +} + +@dynamic resettable5; + +- (NSResettable *)resettable6 { + return 0; // no warning +} +@end + // rdar://problem/19814852 @interface MultiProp @property (nullable, copy) id a, b, c; -- 2.7.4