[cfe-commits] r164047 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseObjc.cpp test/SemaObjC/unused.m test/SemaObjC/warning-missing-selector-name.m
Douglas Gregor
dgregor at apple.com
Mon Sep 17 14:51:43 PDT 2012
On Sep 17, 2012, at 12:15 PM, Fariborz Jahanian <fjahanian at apple.com> 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
> +
> + at interface Super @end
> + at 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;
> + at end
> +
> + at 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::}}
> + at end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list