r350768 - [ObjC] Allow the use of implemented unavailable methods from within

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 11 06:12:52 PST 2019


Here's some user feedback on this new feature.

It looks like the warning is only suppressed if `init` has a definition in
the @interface block. In the 4 cases where we saw this warning fire after
r349841, it still fires after this change because in all 4 cases a class
marked init as unavailable in the @interface but didn't add a definition of
it in the classes @implementation (instead relying on calling the
superclass's (NSObject) init).

It's pretty easy to just add

- (instancetype)init { return [super init]; }

to these 4 classes, but:
a) it doesn't Just Work
b) the diagnostic doesn't make it clear that adding a definition of init
will suppress the warning.

Up to you to decide what (if anything) to do with this feedback :-)

(https://bugs.chromium.org/p/chromium/issues/detail?id=917351#c17 has a
full reduced code snippet.)

On Wed, Jan 9, 2019 at 5:35 PM Alex Lorenz via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: arphaman
> Date: Wed Jan  9 14:31:37 2019
> New Revision: 350768
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350768&view=rev
> Log:
> [ObjC] Allow the use of implemented unavailable methods from within
> the @implementation context
>
> In Objective-C, it's common for some frameworks to mark some methods like
> init
> as unavailable in the @interface to prohibit their usage. However, these
> frameworks then often implemented said method and refer to it in another
> method
> that acts as a factory for that object. The recent change to how messages
> to
> self are type checked in clang (r349841) introduced a regression which
> started
> to prohibit this pattern with an X is unavailable error. This commit
> addresses
> the aforementioned regression.
>
> rdar://47134898
>
> Differential Revision: https://reviews.llvm.org/D56469
>
> Added:
>     cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
> Modified:
>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=350768&r1=350767&r2=350768&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan  9 14:31:37 2019
> @@ -7269,9 +7269,10 @@ ShouldDiagnoseAvailabilityOfDecl(Sema &S
>  /// whether we should emit a diagnostic for \c K and \c DeclVersion in
>  /// the context of \c Ctx. For example, we should emit an unavailable
> diagnostic
>  /// in a deprecated context, but not the other way around.
> -static bool ShouldDiagnoseAvailabilityInContext(Sema &S,
> AvailabilityResult K,
> -                                                VersionTuple DeclVersion,
> -                                                Decl *Ctx) {
> +static bool
> +ShouldDiagnoseAvailabilityInContext(Sema &S, AvailabilityResult K,
> +                                    VersionTuple DeclVersion, Decl *Ctx,
> +                                    const NamedDecl *OffendingDecl) {
>    assert(K != AR_Available && "Expected an unavailable declaration
> here!");
>
>    // Checks if we should emit the availability diagnostic in the context
> of C.
> @@ -7280,9 +7281,22 @@ static bool ShouldDiagnoseAvailabilityIn
>        if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))
>          if (AA->getIntroduced() >= DeclVersion)
>            return true;
> -    } else if (K == AR_Deprecated)
> +    } else if (K == AR_Deprecated) {
>        if (C->isDeprecated())
>          return true;
> +    } else if (K == AR_Unavailable) {
> +      // It is perfectly fine to refer to an 'unavailable' Objective-C
> method
> +      // when it's actually defined and is referenced from within the
> +      // @implementation itself. In this context, we interpret
> unavailable as a
> +      // form of access control.
> +      if (const auto *MD = dyn_cast<ObjCMethodDecl>(OffendingDecl)) {
> +        if (const auto *Impl = dyn_cast<ObjCImplDecl>(C)) {
> +          if (MD->getClassInterface() == Impl->getClassInterface() &&
> +              MD->isDefined())
> +            return true;
> +        }
> +      }
> +    }
>
>      if (C->isUnavailable())
>        return true;
> @@ -7471,7 +7485,8 @@ static void DoEmitAvailabilityWarning(Se
>    if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context,
> OffendingDecl))
>      DeclVersion = AA->getIntroduced();
>
> -  if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))
> +  if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx,
> +                                           OffendingDecl))
>      return;
>
>    SourceLocation Loc = Locs.front();
> @@ -7955,7 +7970,8 @@ void DiagnoseUnguardedAvailability::Diag
>
>      // If the context of this function is less available than D, we
> should not
>      // emit a diagnostic.
> -    if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced,
> Ctx))
> +    if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced,
> Ctx,
> +                                             OffendingDecl))
>        return;
>
>      // We would like to emit the diagnostic even if
> -Wunguarded-availability is
>
> Added: cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m?rev=350768&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m (added)
> +++ cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m Wed Jan  9
> 14:31:37 2019
> @@ -0,0 +1,68 @@
> +// RUN: %clang_cc1 -x objective-c -verify -fobjc-arc %s
> +
> + at interface NSObject
> +
> ++ (instancetype)new;
> ++ (instancetype)alloc;
> +
> + at end
> +
> + at interface Sub: NSObject
> +
> +- (instancetype)init __attribute__((unavailable)); // expected-note 4
> {{'init' has been explicitly marked unavailable here}}
> +
> +- (void)notImplemented __attribute__((unavailable)); // expected-note
> {{'notImplemented' has been explicitly marked unavailable here}}
> +
> + at end
> +
> + at implementation Sub
> +
> ++ (Sub *)create {
> +  return [[self alloc] init];
> +}
> +
> ++ (Sub *)create2 {
> +  return [self new];
> +}
> +
> ++ (Sub *)create3 {
> +  return [Sub new];
> +}
> +
> +- (instancetype) init {
> +  return self;
> +}
> +
> +- (void)reportUseOfUnimplemented {
> +  [self notImplemented]; // expected-error {{'notImplemented' is
> unavailable}}
> +}
> +
> + at end
> +
> + at interface SubClassContext: Sub
> + at end
> +
> + at implementation SubClassContext
> +
> +- (void)subClassContext {
> +  (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}
> +  (void)[Sub new]; // expected-error {{'new' is unavailable}}
> +}
> +
> + at end
> +
> +void unrelatedContext() {
> +  (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}
> +  (void)[Sub new]; // expected-error {{'new' is unavailable}}
> +}
> +
> + at interface X @end
> +
> + at interface X (Foo)
> +-(void)meth __attribute__((unavailable));
> + at end
> +
> + at implementation X (Foo)
> +-(void)meth {}
> +-(void)call_it { [self meth]; }
> + at end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190111/bb6391fe/attachment-0001.html>


More information about the cfe-commits mailing list