On Sep 17, 2012, at 12:15 PM, Fariborz Jahanian <[email protected]> wrote:
> Author: fjahanian > Date: Mon Sep 17 14:15:26 2012 > New Revision: 164047 > > URL: http://llvm.org/viewvc/llvm-project?rev=164047&view=rev > Log: > objective-C: issue warning when there is no whitespace > between objc method parameter name and colon. > // rdar://12263549 > > Added: > cfe/trunk/test/SemaObjC/warning-missing-selector-name.m > Modified: > cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > cfe/trunk/lib/Parse/ParseObjc.cpp > cfe/trunk/test/SemaObjC/unused.m > > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=164047&r1=164046&r2=164047&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Sep 17 14:15:26 > 2012 > @@ -213,6 +213,12 @@ > "expected ';' after static_assert">; > def err_expected_semi_for : Error<"expected ';' in 'for' statement > specifier">; > def err_expected_colon_after : Error<"expected ':' after %0">; > +def missing_selector_name : Warning< > + "parameter name used as selector" > + " may result in incomplete method selector name">, > + InGroup<DiagGroup<"missing-argument-name-in-selector">>; I find this a little hard to parse. How about "%0 used as the name of the previous parameter rather than as part of the selector" > +def note_missing_argument_name : Note< > + "did you mean to use %0 as the selector name instead of %1">; "introduce a parameter name to make %0 part of the selector" and perhaps a note that specifies how to silence this warning, e.g. "remove whitespace after ')' to use %0 the parameter name and have an empty entry in the selector" > def err_label_end_of_compound_statement : Error< > "label at end of compound statement: expected statement">; > def err_address_of_label_outside_fn : Error< > > Modified: cfe/trunk/lib/Parse/ParseObjc.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=164047&r1=164046&r2=164047&view=diff > ============================================================================== > --- cfe/trunk/lib/Parse/ParseObjc.cpp (original) > +++ cfe/trunk/lib/Parse/ParseObjc.cpp Mon Sep 17 14:15:26 2012 > @@ -18,6 +18,7 @@ > #include "clang/Sema/PrettyDeclStackTrace.h" > #include "clang/Sema/Scope.h" > #include "llvm/ADT/SmallVector.h" > +#include "llvm/ADT/StringExtras.h" > using namespace clang; > > > @@ -1031,7 +1032,7 @@ > Scope::FunctionPrototypeScope|Scope::DeclScope); > > AttributePool allParamAttrs(AttrFactory); > - > + bool warnSelectorName = false; > while (1) { > ParsedAttributes paramAttrs(AttrFactory); > Sema::ObjCArgInfo ArgInfo; > @@ -1102,6 +1103,13 @@ > SelIdent = ParseObjCSelectorPiece(selLoc); > if (!SelIdent && Tok.isNot(tok::colon)) > break; > + if (MethodDefinition && !SelIdent) { Why does this only apply to method definitions? > + SourceLocation ColonLoc = Tok.getLocation(); > + if (PP.getLocForEndOfToken(ArgInfo.NameLoc) == ColonLoc) { > + warnSelectorName = true; > + Diag(ArgInfo.NameLoc, diag::missing_selector_name); > + } > + } Also check whether that there is space between the ')' from the previous parameter and the name? That way, we will diagnose - method:(id)foo :blah but we will diagnose - method:(id) foo:blah and we naturally have a way to silence the warning. > // We have a selector or a colon, continue parsing. > } > > @@ -1142,6 +1150,22 @@ > > Selector Sel = PP.getSelectorTable().getSelector(KeyIdents.size(), > &KeyIdents[0]); > + if (warnSelectorName) { > + SmallVector<IdentifierInfo *, 12> DiagKeyIdents; > + for (unsigned i = 0, size = KeyIdents.size(); i < size; i++) > + if (KeyIdents[i]) > + DiagKeyIdents.push_back(KeyIdents[i]); > + else { > + std::string name = "Name"; > + name += llvm::utostr(i+1); > + IdentifierInfo *NamedMissingId = &PP.getIdentifierTable().get(name); > + DiagKeyIdents.push_back(NamedMissingId); > + } > + Selector NewSel = > + PP.getSelectorTable().getSelector(DiagKeyIdents.size(), > &DiagKeyIdents[0]); > + Diag(mLoc, diag::note_missing_argument_name) > + << NewSel.getAsString() << Sel.getAsString(); > + } These notes are very far away from the warning they should conceptually be attached to, and there could certainly be diagnostics in-between. How about either using the note up into the chunk above, where the warning is emitted (I think the note text I suggested will work), or (if you really need the full selector) delaying the warnings until we get here? > Decl *Result > = Actions.ActOnMethodDeclaration(getCurScope(), mLoc, > Tok.getLocation(), > mType, DSRet, ReturnType, > > Modified: cfe/trunk/test/SemaObjC/unused.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unused.m?rev=164047&r1=164046&r2=164047&view=diff > ============================================================================== > --- cfe/trunk/test/SemaObjC/unused.m (original) > +++ cfe/trunk/test/SemaObjC/unused.m Mon Sep 17 14:15:26 2012 > @@ -29,8 +29,9 @@ > @end > > @implementation foo > -- (int) meth: (int)x: > -(int)y: // expected-warning{{unused}} > +- (int) meth: (int)x: // expected-warning {{parameter name used as selector > may result in incomplete method selector name}} \ > + // expected-note {{did you mean to use > meth:Name2:Name3: as the selector name instead of meth:::}} > +(int)y: // expected-warning{{unused}} expected-warning {{parameter name > used as selector may result in incomplete method selector name}} > (int) __attribute__((unused))z { return x; } > @end > > > Added: cfe/trunk/test/SemaObjC/warning-missing-selector-name.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/warning-missing-selector-name.m?rev=164047&view=auto > ============================================================================== > --- cfe/trunk/test/SemaObjC/warning-missing-selector-name.m (added) > +++ cfe/trunk/test/SemaObjC/warning-missing-selector-name.m Mon Sep 17 > 14:15:26 2012 > @@ -0,0 +1,20 @@ > +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s > +// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -verify > -Wno-objc-root-class -Wmissing-argument-name-in-selector %s > +// rdar://12263549 > + > +@interface Super @end > +@interface INTF : Super > +-(void) Name1:(id)Arg1 Name2:(id)Arg2; // Name1:Name2: > +-(void) Name1:(id) Name2:(id)Arg2; > +-(void) Name1:(id)Arg1 Name2:(id)Arg2 Name3:(id)Arg3; // Name1:Name2:Name3: > +-(void) Name1:(id)Arg1 Name2:(id) Name3:(id)Arg3; > +@end > + > +@implementation INTF > +-(void) Name1:(id)Arg1 Name2:(id)Arg2{} > +-(void) Name1:(id) Name2:(id)Arg2 {} // expected-warning {{parameter name > used as selector may result in incomplete method selector name}} \ > + // expected-note {{did you mean to use > Name1:Name2: as the selector name instead of Name1::}} > +-(void) Name1:(id)Arg1 Name2:(id)Arg2 Name3:(id)Arg3 {} > +-(void) Name1:(id)Arg1 Name2:(id) Name3:(id)Arg3 {} // expected-warning > {{parameter name used as selector may result in incomplete method selector > name}} \ > + // expected-note {{did you > mean to use Name1:Name2:Name3: as the selector name instead of Name1:Name2::}} > +@end > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
