[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 15:11:20 PDT 2012
On Sep 17, 2012, at 3:09 PM, jahanian <fjahanian at apple.com> wrote:
>
> On Sep 17, 2012, at 3:04 PM, Douglas Gregor wrote:
>
>>
>> On Sep 17, 2012, at 3:01 PM, jahanian <fjahanian at apple.com> wrote:
>>
>>>
>>> On Sep 17, 2012, at 2:51 PM, Douglas Gregor wrote:
>>>
>>>>
>>>> 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?
>>>
>>> It only concerns the class implementor, so it goes in method definition.
>>> Putting it in method declaration creates warnings that users of the method have no control over.
>>
>> So we wouldn't diagnose the frightening problem in this code?
>>
>> @interface A
>> - method:(id) second:(id);
>> @end
>
> Every method type requires a parameter name. So, this is illegal to start with.
>
> % $CLANG -c t.m
> t.m:2:26: error: expected identifier
> - method:(id) second:(id);
> ^
> 1 error generated.
> - fariborz
Corrected example:
interface A
- method:(id) second:(id)second;
@end
@implementation A
- method:(id)first second:(id)second { }
@end
Aside from the general "incomplete implementation" warning, this won't get diagnosed by your patch because it's the @interface that has the problem, but we're only diagnosing for @implementations.
- Doug
More information about the cfe-commits
mailing list