On Fri, Jun 19, 2015 at 2:14 PM, Douglas Gregor <[email protected]> wrote: > Author: dgregor > Date: Fri Jun 19 13:14:46 2015 > New Revision: 240155 > > URL: http://llvm.org/viewvc/llvm-project?rev=240155&view=rev > Log: > 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. > > Modified: > cfe/trunk/include/clang/AST/DeclObjC.h > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/include/clang/Sema/DeclSpec.h > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/AST/DeclPrinter.cpp > cfe/trunk/lib/Parse/ParseObjc.cpp > cfe/trunk/lib/Sema/SemaDeclObjC.cpp > cfe/trunk/lib/Sema/SemaObjCProperty.cpp > cfe/trunk/test/SemaObjC/nullability.m > > Modified: cfe/trunk/include/clang/AST/DeclObjC.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/DeclObjC.h (original) > +++ cfe/trunk/include/clang/AST/DeclObjC.h Fri Jun 19 13:14:46 2015 > @@ -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 }; > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 19 13:14:46 > 2015 > @@ -7714,6 +7714,10 @@ def note_nullability_type_specifier : No > "'%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<Nullability>; > + > } > > } // end of sema component. > > Modified: cfe/trunk/include/clang/Sema/DeclSpec.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/DeclSpec.h (original) > +++ cfe/trunk/include/clang/Sema/DeclSpec.h Fri Jun 19 13:14:46 2015 > @@ -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; > > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jun 19 13:14:46 2015 > @@ -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, > > Modified: cfe/trunk/lib/AST/DeclPrinter.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclPrinter.cpp?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/DeclPrinter.cpp (original) > +++ cfe/trunk/lib/AST/DeclPrinter.cpp Fri Jun 19 13:14:46 2015 > @@ -1213,8 +1213,14 @@ void DeclPrinter::VisitObjCPropertyDecl( > 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); > + }
Elide braces. > first = false; > } > } > > Modified: cfe/trunk/lib/Parse/ParseObjc.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/lib/Parse/ParseObjc.cpp (original) > +++ cfe/trunk/lib/Parse/ParseObjc.cpp Fri Jun 19 13:14:46 2015 > @@ -611,6 +611,7 @@ static void diagnoseRedundantPropertyNul > /// nonnull > /// nullable > /// null_unspecified > +/// null_resettable > /// > void Parser::ParseObjCPropertyAttribute(ObjCDeclSpec &DS) { > assert(Tok.getKind() == tok::l_paren); > @@ -717,6 +718,16 @@ void Parser::ParseObjCPropertyAttribute( > 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); > > Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Fri Jun 19 13:14:46 2015 > @@ -2024,6 +2024,9 @@ void Sema::ImplMethodsVsClassMethods(Sco > 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()); > > Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original) > +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Fri Jun 19 13:14:46 2015 > @@ -361,6 +361,9 @@ Sema::HandlePropertyInClassExtension(Sco > 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::CreatePropertyDe > 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::DiagnoseUnimplementedProperti > } > } > > +void Sema::diagnoseNullResettableSynthesizedSetters(ObjCImplDecl *impDecl) { impDecl can be a const pointer, can't it? > + 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) && The more I see the bitwise tests, the more I wonder if having a propertyAttributeIsSet() function might make sense? > + 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(); Under what circumstances would loc be invalid here? (I may simply be unfamiliar with ObjC and that's not a problem.) > + > + 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(ObjCPrope > 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(ObjCPrope > 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)){ Missing a space before the { (which could be elided, but I kind of personally prefer to keep). > + 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); > > Modified: cfe/trunk/test/SemaObjC/nullability.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nullability.m?rev=240155&r1=240154&r2=240155&view=diff > ============================================================================== > --- cfe/trunk/test/SemaObjC/nullability.m (original) > +++ cfe/trunk/test/SemaObjC/nullability.m Fri Jun 19 13:14:46 2015 > @@ -164,6 +164,45 @@ void test_instancetype(InitializableClas > 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; > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
