<div dir="ltr">Hi,<div><br></div><div>We are planning to fix this issue by not checking if the method is defined. I will post a patch this week.</div><div><br></div><div>Cheers,</div><div>Alex</div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, 11 Jan 2019 at 10:26, Alex L <<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thanks, we might have similar cases in our code base as well. We'll see if we can fix that too.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, 11 Jan 2019 at 06:13, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">Here's some user feedback on this new feature.<div><br></div><div>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).</div><div><br></div><div>It's pretty easy to just add</div><div><br></div><div>- (instancetype)init { return [super init]; }</div><div><br></div><div>to these 4 classes, but:</div><div>a) it doesn't Just Work</div><div>b) the diagnostic doesn't make it clear that adding a definition of init will suppress the warning.</div><div><br></div><div>Up to you to decide what (if anything) to do with this feedback :-)</div><div><br></div><div>(<a href="https://bugs.chromium.org/p/chromium/issues/detail?id=917351#c17" target="_blank">https://bugs.chromium.org/p/chromium/issues/detail?id=917351#c17</a> has a full reduced code snippet.)</div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jan 9, 2019 at 5:35 PM Alex Lorenz via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Author: arphaman<br>
Date: Wed Jan 9 14:31:37 2019<br>
New Revision: 350768<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=350768&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=350768&view=rev</a><br>
Log:<br>
[ObjC] Allow the use of implemented unavailable methods from within<br>
the @implementation context<br>
<br>
In Objective-C, it's common for some frameworks to mark some methods like init<br>
as unavailable in the @interface to prohibit their usage. However, these<br>
frameworks then often implemented said method and refer to it in another method<br>
that acts as a factory for that object. The recent change to how messages to<br>
self are type checked in clang (r349841) introduced a regression which started<br>
to prohibit this pattern with an X is unavailable error. This commit addresses<br>
the aforementioned regression.<br>
<br>
rdar://47134898<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D56469" rel="noreferrer" target="_blank">https://reviews.llvm.org/D56469</a><br>
<br>
Added:<br>
cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=350768&r1=350767&r2=350768&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=350768&r1=350767&r2=350768&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan 9 14:31:37 2019<br>
@@ -7269,9 +7269,10 @@ ShouldDiagnoseAvailabilityOfDecl(Sema &S<br>
/// whether we should emit a diagnostic for \c K and \c DeclVersion in<br>
/// the context of \c Ctx. For example, we should emit an unavailable diagnostic<br>
/// in a deprecated context, but not the other way around.<br>
-static bool ShouldDiagnoseAvailabilityInContext(Sema &S, AvailabilityResult K,<br>
- VersionTuple DeclVersion,<br>
- Decl *Ctx) {<br>
+static bool<br>
+ShouldDiagnoseAvailabilityInContext(Sema &S, AvailabilityResult K,<br>
+ VersionTuple DeclVersion, Decl *Ctx,<br>
+ const NamedDecl *OffendingDecl) {<br>
assert(K != AR_Available && "Expected an unavailable declaration here!");<br>
<br>
// Checks if we should emit the availability diagnostic in the context of C.<br>
@@ -7280,9 +7281,22 @@ static bool ShouldDiagnoseAvailabilityIn<br>
if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))<br>
if (AA->getIntroduced() >= DeclVersion)<br>
return true;<br>
- } else if (K == AR_Deprecated)<br>
+ } else if (K == AR_Deprecated) {<br>
if (C->isDeprecated())<br>
return true;<br>
+ } else if (K == AR_Unavailable) {<br>
+ // It is perfectly fine to refer to an 'unavailable' Objective-C method<br>
+ // when it's actually defined and is referenced from within the<br>
+ // @implementation itself. In this context, we interpret unavailable as a<br>
+ // form of access control.<br>
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(OffendingDecl)) {<br>
+ if (const auto *Impl = dyn_cast<ObjCImplDecl>(C)) {<br>
+ if (MD->getClassInterface() == Impl->getClassInterface() &&<br>
+ MD->isDefined())<br>
+ return true;<br>
+ }<br>
+ }<br>
+ }<br>
<br>
if (C->isUnavailable())<br>
return true;<br>
@@ -7471,7 +7485,8 @@ static void DoEmitAvailabilityWarning(Se<br>
if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, OffendingDecl))<br>
DeclVersion = AA->getIntroduced();<br>
<br>
- if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))<br>
+ if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx,<br>
+ OffendingDecl))<br>
return;<br>
<br>
SourceLocation Loc = Locs.front();<br>
@@ -7955,7 +7970,8 @@ void DiagnoseUnguardedAvailability::Diag<br>
<br>
// If the context of this function is less available than D, we should not<br>
// emit a diagnostic.<br>
- if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, Ctx))<br>
+ if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, Ctx,<br>
+ OffendingDecl))<br>
return;<br>
<br>
// We would like to emit the diagnostic even if -Wunguarded-availability is<br>
<br>
Added: cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m?rev=350768&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m?rev=350768&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m (added)<br>
+++ cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m Wed Jan 9 14:31:37 2019<br>
@@ -0,0 +1,68 @@<br>
+// RUN: %clang_cc1 -x objective-c -verify -fobjc-arc %s<br>
+<br>
+@interface NSObject<br>
+<br>
++ (instancetype)new;<br>
++ (instancetype)alloc;<br>
+<br>
+@end<br>
+<br>
+@interface Sub: NSObject<br>
+<br>
+- (instancetype)init __attribute__((unavailable)); // expected-note 4 {{'init' has been explicitly marked unavailable here}}<br>
+<br>
+- (void)notImplemented __attribute__((unavailable)); // expected-note {{'notImplemented' has been explicitly marked unavailable here}}<br>
+<br>
+@end<br>
+<br>
+@implementation Sub<br>
+<br>
++ (Sub *)create {<br>
+ return [[self alloc] init];<br>
+}<br>
+<br>
++ (Sub *)create2 {<br>
+ return [self new];<br>
+}<br>
+<br>
++ (Sub *)create3 {<br>
+ return [Sub new];<br>
+}<br>
+<br>
+- (instancetype) init {<br>
+ return self;<br>
+}<br>
+<br>
+- (void)reportUseOfUnimplemented {<br>
+ [self notImplemented]; // expected-error {{'notImplemented' is unavailable}}<br>
+}<br>
+<br>
+@end<br>
+<br>
+@interface SubClassContext: Sub<br>
+@end<br>
+<br>
+@implementation SubClassContext<br>
+<br>
+- (void)subClassContext {<br>
+ (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}<br>
+ (void)[Sub new]; // expected-error {{'new' is unavailable}}<br>
+}<br>
+<br>
+@end<br>
+<br>
+void unrelatedContext() {<br>
+ (void)[[Sub alloc] init]; // expected-error {{'init' is unavailable}}<br>
+ (void)[Sub new]; // expected-error {{'new' is unavailable}}<br>
+}<br>
+<br>
+@interface X @end<br>
+<br>
+@interface X (Foo)<br>
+-(void)meth __attribute__((unavailable));<br>
+@end<br>
+<br>
+@implementation X (Foo)<br>
+-(void)meth {}<br>
+-(void)call_it { [self meth]; }<br>
+@end<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>