objective-C: improve on warnings about misplacement of method

argument names. // rdar://12263549


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@164077 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 34 insertions, 35 deletions

diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
index 218a6a31da..6d308ff70d 100644
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -213,10 +213,15 @@ def err_expected_semi_after_static_assert : Error<
   "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">>;
+def warn_missing_selector_name : Warning<
+  "%0 used as the name of the previous parameter rather than as part "
+  "of the selector">,
+  InGroup<DiagGroup<"missing-selector-name">>;
+def note_missing_selector_name : Note<
+  "introduce a parameter name to make %0 part of the selector">;
+def note_force_empty_selector_name : Note<
+  "or insert whitespace before ':' to use %0 as parameter name "
+  "and have an empty entry in the selector">;
 def note_missing_argument_name : Note<
   "did you mean to use %0 as the selector name instead of %1">;
 def err_label_end_of_compound_statement : Error<
diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp
index 48c5efac6b..d321baf836 100644
--- a/lib/Parse/ParseObjc.cpp
+++ b/lib/Parse/ParseObjc.cpp
@@ -1032,7 +1032,6 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
                             Scope::FunctionPrototypeScope|Scope::DeclScope);
 
   AttributePool allParamAttrs(AttrFactory);
-  bool warnSelectorName = false;
   while (1) {
     ParsedAttributes paramAttrs(AttrFactory);
     Sema::ObjCArgInfo ArgInfo;
@@ -1103,11 +1102,12 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
     SelIdent = ParseObjCSelectorPiece(selLoc);
     if (!SelIdent && Tok.isNot(tok::colon))
       break;
-    if (MethodDefinition && !SelIdent) {
+    if (!SelIdent) {
       SourceLocation ColonLoc = Tok.getLocation();
       if (PP.getLocForEndOfToken(ArgInfo.NameLoc) == ColonLoc) {
-        warnSelectorName = true;
-        Diag(ArgInfo.NameLoc, diag::missing_selector_name);
+        Diag(ArgInfo.NameLoc, diag::warn_missing_selector_name) << ArgInfo.Name;
+        Diag(ArgInfo.NameLoc, diag::note_missing_selector_name) << ArgInfo.Name;
+        Diag(ColonLoc, diag::note_force_empty_selector_name) << ArgInfo.Name;
       }
     }
     // We have a selector or a colon, continue parsing.
@@ -1150,22 +1150,6 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
   
   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();
-  }
   Decl *Result
        = Actions.ActOnMethodDeclaration(getCurScope(), mLoc, Tok.getLocation(),
                                         mType, DSRet, ReturnType, 
diff --git a/test/SemaObjC/unused.m b/test/SemaObjC/unused.m
index 23d7e8cbe5..3fd1cf0467 100644
--- a/test/SemaObjC/unused.m
+++ b/test/SemaObjC/unused.m
@@ -25,13 +25,17 @@ void test2() {
 }
 
 @interface foo
-- (int)meth: (int)x: (int)y: (int)z ;
+- (int)meth: (int)x : (int)y : (int)z ;
 @end
 
 @implementation foo
-- (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) meth: (int)x: // expected-warning {{'x' used as the name of the previous parameter rather than as part of the selector}} \
+                      // expected-note {{introduce a parameter name to make 'x' part of the selector}} \
+                      // expected-note {{or insert whitespace before ':' to use 'x' as parameter name and have an empty entry in the selector}}
+
+(int)y:  // expected-warning {{unused}}  expected-warning {{'y' used as the name of the previous parameter rather than as part of the selector}} \
+         // expected-note {{introduce a parameter name to make 'y' part of the selector}} \
+         // expected-note {{or insert whitespace before ':' to use 'y' as parameter name and have an empty entry in the selector}}
 (int) __attribute__((unused))z { return x; }
 @end
 
diff --git a/test/SemaObjC/warning-missing-selector-name.m b/test/SemaObjC/warning-missing-selector-name.m
index e4bde0b746..095702343f 100644
--- a/test/SemaObjC/warning-missing-selector-name.m
+++ b/test/SemaObjC/warning-missing-selector-name.m
@@ -1,20 +1,26 @@
 // 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
+// RUN: %clang_cc1 -x objective-c++ -fsyntax-only -verify -Wno-objc-root-class -Wmissing-selector-name %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) Name2:(id)Arg2; // expected-warning {{'Name2' used as the name of the previous parameter rather than as part of the selector}} \
+				   // expected-note {{introduce a parameter name to make 'Name2' part of the selector}} \
+				   // expected-note {{or insert whitespace before ':' to use 'Name2' as parameter name and have an empty entry in the selector}}
 -(void) Name1:(id)Arg1 Name2:(id)Arg2 Name3:(id)Arg3; // Name1:Name2:Name3:
--(void) Name1:(id)Arg1 Name2:(id) Name3:(id)Arg3;
+-(void) Name1:(id)Arg1 Name2:(id) Name3:(id)Arg3; // expected-warning {{'Name3' used as the name of the previous parameter rather than as part of the selector}} \
+				   // expected-note {{introduce a parameter name to make 'Name3' part of the selector}} \
+				   // expected-note {{or insert whitespace before ':' to use 'Name3' as parameter name and have an empty entry in the selector}}
 @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) Name2:(id)Arg2 {} // expected-warning {{'Name2' used as the name of the previous parameter rather than as part of the selector}} \
+					// expected-note {{introduce a parameter name to make 'Name2' part of the selector}} \
+ 					// expected-note {{or insert whitespace before ':' to use 'Name2' as parameter name and have an empty entry in the selector}}
 -(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::}}
+-(void) Name1:(id)Arg1 Name2:(id) Name3:(id)Arg3 {} // expected-warning {{'Name3' used as the name of the previous parameter rather than as part of the selector}} \
+					// expected-note {{introduce a parameter name to make 'Name3' part of the selector}} \
+ 					// expected-note {{or insert whitespace before ':' to use 'Name3' as parameter name and have an empty entry in the selector}}
 @end