r203693 - Objective-C. Issue diagnostics on mismatched methods when their selector is used
jahanian
fjahanian at apple.com
Thu Mar 27 10:29:18 PDT 2014
On Mar 27, 2014, at 10:01 AM, Nico Weber <thakis at google.com> wrote:
> Isn't a warning where the suggested fix is "turn off the warning" kind of pointless?
There are cases that this warning could be false positive. And there are cases that it points to unexpected
runtime behavior. I think the test below is one of them. But, this warning should be opted in.
Currently, it is on by default. I will make the change so it is only opted in.
- Fariborz
>
>
> On Thu, Mar 27, 2014 at 9:55 AM, jahanian <fjahanian at apple.com> wrote:
> Currently, we only note two methods in the warning. It can be improved to note all methods. - Fariborz
>
> On Mar 26, 2014, at 9:40 PM, Nico Weber <thakis at google.com> wrote:
>
>> And a second question: In this example, clang only prints the save: calls in system headers but not the one in the interface that's actually the one that will be called at runtime:
>>
>> Nicos-MacBook-Pro:src thakis$ cat test.mm
>> #import <Cocoa/Cocoa.h>
>>
>> @interface WindowSizeAutosaver : NSObject
>> @end
>>
>> @interface WindowSizeAutosaver (Private)
>> - (void)save:(NSNotification*)notification;
>> @end
>>
>> @implementation WindowSizeAutosaver
>> - (id)initWithWindow:(NSWindow*)window
>> path:(const char*)path {
>> if ((self = [super init])) {
>> [[NSNotificationCenter defaultCenter]
>> addObserver:self
>> selector:@selector(save:)
>> name:NSWindowDidMoveNotification
>> object:window];
>> }
>> return self;
>> }
>> @end
>> Nicos-MacBook-Pro:src thakis$ ~/src/llvm-build/bin/clang -c test.mm -Wall -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk
>> test.mm:16:19: warning: several methods with selector 'save:' of mismatched types are found for the @selector expression [-Wselector-type-mismatch]
>> selector:@selector(save:)
>> ^
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSUserDefaultsController.h:43:1: note: method 'save:' declared here
>> - (void)save:(id)sender; // no effect if applies immediately
>> ^
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/System/Library/Frameworks/CoreData.framework/Headers/NSManagedObjectContext.h:185:1: note: method 'save:' declared here
>> - (BOOL)save:(NSError **)error;
>> ^
>> 1 warning generated.
>>
>>
>> On Wed, Mar 26, 2014 at 9:22 PM, Nico Weber <thakis at chromium.org> wrote:
>> Hi,
>>
>> how does one fix this warning? I have this code:
>>
>> NSString* string = @"";
>> [NSThread detachNewThreadSelector:@selector(length)
>> toTarget:string
>> withObject:nil];
>>
>>
>> clang now warns:
>>
>> ../../content/renderer/renderer_main_platform_delegate_mac.mm:122:39: error: several methods with selector 'length' of mismatched types are found for the @selector expression [-Werror,-Wselector-type-mismatch]
>> [NSThread detachNewThreadSelector:@selector(length)
>> ^
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSString.h:78:1: note: method 'length' declared here
>> - (NSUInteger)length;
>> ^
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSStatusItem.h:45:1: note: method 'length' declared here
>> - (CGFloat)length;
>> ^
>>
>> What am I supposed to do about this?
>>
>>
>>
>> On Wed, Mar 12, 2014 at 11:34 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>> Author: fjahanian
>> Date: Wed Mar 12 13:34:01 2014
>> New Revision: 203693
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=203693&view=rev
>> Log:
>> Objective-C. Issue diagnostics on mismatched methods when their selector is used
>> in an @selector expression. // rdar://15794055
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaExprObjC.cpp
>> cfe/trunk/test/SemaObjC/selector-1.m
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=203693&r1=203692&r2=203693&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar 12 13:34:01 2014
>> @@ -892,7 +892,9 @@ def warn_unimplemented_selector: Warnin
>> InGroup<Selector>, DefaultIgnore;
>> def warn_unimplemented_protocol_method : Warning<
>> "method %0 in protocol %1 not implemented">, InGroup<Protocol>;
>> -
>> +def warning_multiple_selectors: Warning<
>> + "several methods with selector %0 of mismatched types are found for the @selector expression">,
>> + InGroup<SelectorTypeMismatch>;
>> // C++ declarations
>> def err_static_assert_expression_is_not_constant : Error<
>> "static_assert expression is not an integral constant expression">;
>>
>> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=203693&r1=203692&r2=203693&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Wed Mar 12 13:34:01 2014
>> @@ -973,6 +973,58 @@ ExprResult Sema::ParseObjCEncodeExpressi
>> return BuildObjCEncodeExpression(AtLoc, TInfo, RParenLoc);
>> }
>>
>> +static bool HelperToDiagnoseMismatchedMethodsInGlobalPool(Sema &S,
>> + SourceLocation AtLoc,
>> + ObjCMethodDecl *Method,
>> + ObjCMethodList &MethList) {
>> + ObjCMethodList *M = &MethList;
>> + bool Warned = false;
>> + for (M = M->getNext(); M; M=M->getNext()) {
>> + ObjCMethodDecl *MatchingMethodDecl = M->Method;
>> + if (MatchingMethodDecl == Method ||
>> + isa<ObjCImplDecl>(MatchingMethodDecl->getDeclContext()) ||
>> + MatchingMethodDecl->getSelector() != Method->getSelector())
>> + continue;
>> + if (!S.MatchTwoMethodDeclarations(Method,
>> + MatchingMethodDecl, Sema::MMS_loose)) {
>> + if (!Warned) {
>> + Warned = true;
>> + S.Diag(AtLoc, diag::warning_multiple_selectors)
>> + << Method->getSelector();
>> + S.Diag(Method->getLocation(), diag::note_method_declared_at)
>> + << Method->getDeclName();
>> + }
>> + S.Diag(MatchingMethodDecl->getLocation(), diag::note_method_declared_at)
>> + << MatchingMethodDecl->getDeclName();
>> + }
>> + }
>> + return Warned;
>> +}
>> +
>> +static void DiagnoseMismatchedSelectors(Sema &S, SourceLocation AtLoc,
>> + ObjCMethodDecl *Method) {
>> + unsigned DIAG = diag::warning_multiple_selectors;
>> + if (S.Diags.getDiagnosticLevel(DIAG, SourceLocation())
>> + == DiagnosticsEngine::Ignored)
>> + return;
>> + bool Warned = false;
>> + for (Sema::GlobalMethodPool::iterator b = S.MethodPool.begin(),
>> + e = S.MethodPool.end(); b != e; b++) {
>> + // first, instance methods
>> + ObjCMethodList &InstMethList = b->second.first;
>> + if (HelperToDiagnoseMismatchedMethodsInGlobalPool(S, AtLoc,
>> + Method, InstMethList))
>> + Warned = true;
>> +
>> + // second, class methods
>> + ObjCMethodList &ClsMethList = b->second.second;
>> + if (HelperToDiagnoseMismatchedMethodsInGlobalPool(S, AtLoc,
>> + Method, ClsMethList) ||
>> + Warned)
>> + return;
>> + }
>> +}
>> +
>> ExprResult Sema::ParseObjCSelectorExpression(Selector Sel,
>> SourceLocation AtLoc,
>> SourceLocation SelLoc,
>> @@ -994,7 +1046,8 @@ ExprResult Sema::ParseObjCSelectorExpres
>>
>> } else
>> Diag(SelLoc, diag::warn_undeclared_selector) << Sel;
>> - }
>> + } else
>> + DiagnoseMismatchedSelectors(*this, AtLoc, Method);
>>
>> if (!Method ||
>> Method->getImplementationControl() != ObjCMethodDecl::Optional) {
>>
>> Modified: cfe/trunk/test/SemaObjC/selector-1.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/selector-1.m?rev=203693&r1=203692&r2=203693&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/selector-1.m (original)
>> +++ cfe/trunk/test/SemaObjC/selector-1.m Wed Mar 12 13:34:01 2014
>> @@ -1,18 +1,17 @@
>> // RUN: %clang_cc1 -verify %s
>> -// expected-no-diagnostics
>>
>> @interface I
>> -- (id) compare: (char) arg1;
>> +- (id) compare: (char) arg1; // expected-note {{method 'compare:' declared here}}
>> - length;
>> @end
>>
>> @interface J
>> -- (id) compare: (id) arg1;
>> +- (id) compare: (id) arg1; // expected-note {{method 'compare:' declared here}}
>> @end
>>
>> SEL func()
>> {
>> - return @selector(compare:); // no warning on multiple selector found.
>> + return @selector(compare:); // expected-warning {{several methods with selector 'compare:' of mismatched types are found for the @selector expression}}
>> }
>>
>> int main() {
>> @@ -27,3 +26,26 @@ int main() {
>>
>> SEL s9 = @selector(:enum:bool:);
>> }
>> +
>> +// rdar://15794055
>> + at interface NSObject @end
>> +
>> + at class NSNumber;
>> +
>> + at interface XBRecipe : NSObject
>> + at property (nonatomic, assign) float finalVolume; // expected-note {{method 'setFinalVolume:' declared here}}
>> + at end
>> +
>> + at interface XBDocument : NSObject
>> + at end
>> +
>> + at interface XBDocument ()
>> +- (void)setFinalVolume:(NSNumber *)finalVolumeNumber; // expected-note {{method 'setFinalVolume:' declared here}}
>> + at end
>> +
>> + at implementation XBDocument
>> +- (void)setFinalVolume:(NSNumber *)finalVolumeNumber
>> +{
>> + (void)@selector(setFinalVolume:); // expected-warning {{several methods with selector 'setFinalVolume:' of mismatched types are found for the @selector expression}}
>> +}
>> + at end
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140327/16c528dc/attachment.html>
More information about the cfe-commits
mailing list