r240155 - Implement the 'null_resettable' attribute for Objective-C properties.
Aaron Ballman
aaron at aaronballman.com
Sat Jun 20 12:10:21 PDT 2015
On Fri, Jun 19, 2015 at 2:14 PM, Douglas Gregor <dgregor at apple.com> 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))
> + at interface NSResettable
> + at property(null_resettable,retain) NSResettable *resettable1; // expected-note{{passing argument to parameter 'resettable1' here}}
> + at property(null_resettable,retain,nonatomic) NSResettable *resettable2;
> + at property(null_resettable,retain,nonatomic) NSResettable *resettable3;
> + at property(null_resettable,retain,nonatomic) NSResettable *resettable4;
> + at property(null_resettable,retain,nonatomic) NSResettable *resettable5;
> + at property(null_resettable,retain,nonatomic) NSResettable *resettable6;
> + at 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 *'}}
> +}
> +
> + at 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 {
> +}
> +
> + at synthesize resettable2; // no warning; not synthesized
> + at synthesize resettable3; // expected-warning{{synthesized setter 'setResettable3:' for null_resettable property 'resettable3' does not handle nil}}
> +
> +- (void)setResettable2:(NSResettable *)param {
> +}
> +
> + at dynamic resettable5;
> +
> +- (NSResettable *)resettable6 {
> + return 0; // no warning
> +}
> + at end
> +
> // rdar://problem/19814852
> @interface MultiProp
> @property (nullable, copy) id a, b, c;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
~Aaron
More information about the cfe-commits
mailing list