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

Alex L via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 11 10:26:56 PST 2019


Thanks, we might have similar cases in our code base as well. We'll see if
we can fix that too.

On Fri, 11 Jan 2019 at 06:13, Nico Weber <thakis at chromium.org> wrote:

> 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/ff9e7481/attachment-0001.html>


More information about the cfe-commits mailing list