r350768 - [ObjC] Allow the use of implemented unavailable methods from within
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 17 07:57:39 PST 2019
For the archives, here's said patch: https://reviews.llvm.org/D56816
(Thanks!)
On Wed, Jan 16, 2019 at 5:26 PM Alex L <arphaman at gmail.com> wrote:
> 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 <arphaman at 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 <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/20190117/6cd8e1c1/attachment-0001.html>
More information about the cfe-commits
mailing list