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