Hi,
We are planning to fix this issue by not checking if the method is defined.
I will post a patch this week.
Cheers,
Alex
On Fri, 11 Jan 2019 at 10:26, Alex L <arpha...@gmail.com> wrote:
> 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 <tha...@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@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
>>> +
>>> +@interface NSObject
>>> +
>>> ++ (instancetype)new;
>>> ++ (instancetype)alloc;
>>> +
>>> +@end
>>> +
>>> +@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}}
>>> +
>>> +@end
>>> +
>>> +@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}}
>>> +}
>>> +
>>> +@end
>>> +
>>> +@interface SubClassContext: Sub
>>> +@end
>>> +
>>> +@implementation SubClassContext
>>> +
>>> +- (void)subClassContext {
>>> + (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}
>>> + (void)[Sub new]; // expected-error {{'new' is unavailable}}
>>> +}
>>> +
>>> +@end
>>> +
>>> +void unrelatedContext() {
>>> + (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}
>>> + (void)[Sub new]; // expected-error {{'new' is unavailable}}
>>> +}
>>> +
>>> +@interface X @end
>>> +
>>> +@interface X (Foo)
>>> +-(void)meth __attribute__((unavailable));
>>> +@end
>>> +
>>> +@implementation X (Foo)
>>> +-(void)meth {}
>>> +-(void)call_it { [self meth]; }
>>> +@end
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
