Better diagnostic for superfluous scope specifier inside a class definition for member functions. + Fixit.

Example: 
class A {
   void A::foo(); //warning: extra qualification on member 'foo'
};


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

5 files changed, 64 insertions, 43 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index e6bcd86609..42b22f4f55 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2140,6 +2140,8 @@ def err_qualified_param_declarator : Error<
 def ext_out_of_line_declaration : ExtWarn<
   "out-of-line declaration of a member must be a definition">,
   InGroup<OutOfLineDeclaration>, DefaultError;
+def warn_member_extra_qualification : Warning<
+  "extra qualification on member %0">;
 def note_member_def_close_match : Note<"member declaration nearly matches">;
 def err_typecheck_ivar_variable_size : Error<
   "instance variables must have a constant size">;
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 7902ce84b5..f0a0e67f92 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -3687,51 +3687,61 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   }
 
   if (D.getCXXScopeSpec().isSet() && !NewFD->isInvalidDecl()) {
-    // Fake up an access specifier if it's supposed to be a class member.
-    if (!Redeclaration && isa<CXXRecordDecl>(NewFD->getDeclContext()))
-      NewFD->setAccess(AS_public);
-
-    // An out-of-line member function declaration must also be a
-    // definition (C++ [dcl.meaning]p1).
-    // Note that this is not the case for explicit specializations of
-    // function templates or member functions of class templates, per
-    // C++ [temp.expl.spec]p2. We also allow these declarations as an extension
-    // for compatibility with old SWIG code which likes to generate them.
-    if (!IsFunctionDefinition && !isFriend &&
-        !isFunctionTemplateSpecialization && !isExplicitSpecialization) {
-      Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
-        << D.getCXXScopeSpec().getRange();
-    }
-    if (!Redeclaration && !(isFriend && CurContext->isDependentContext())) {
-      // The user tried to provide an out-of-line definition for a
-      // function that is a member of a class or namespace, but there
-      // was no such member function declared (C++ [class.mfct]p2,
-      // C++ [namespace.memdef]p2). For example:
+    if (!CurContext->isRecord()) {
+      // Fake up an access specifier if it's supposed to be a class member.
+      if (!Redeclaration && isa<CXXRecordDecl>(NewFD->getDeclContext()))
+        NewFD->setAccess(AS_public);
+
+      // An out-of-line member function declaration must also be a
+      // definition (C++ [dcl.meaning]p1).
+      // Note that this is not the case for explicit specializations of
+      // function templates or member functions of class templates, per
+      // C++ [temp.expl.spec]p2. We also allow these declarations as an extension
+      // for compatibility with old SWIG code which likes to generate them.
+      if (!IsFunctionDefinition && !isFriend &&
+          !isFunctionTemplateSpecialization && !isExplicitSpecialization) {
+        Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
+          << D.getCXXScopeSpec().getRange();
+      }
+      if (!Redeclaration && !(isFriend && CurContext->isDependentContext())) {
+        // The user tried to provide an out-of-line definition for a
+        // function that is a member of a class or namespace, but there
+        // was no such member function declared (C++ [class.mfct]p2,
+        // C++ [namespace.memdef]p2). For example:
+        //
+        // class X {
+        //   void f() const;
+        // };
+        //
+        // void X::f() { } // ill-formed
+        //
+        // Complain about this problem, and attempt to suggest close
+        // matches (e.g., those that differ only in cv-qualifiers and
+        // whether the parameter types are references).
+        Diag(D.getIdentifierLoc(), diag::err_member_def_does_not_match)
+          << Name << DC << D.getCXXScopeSpec().getRange();
+        NewFD->setInvalidDecl();
+
+        LookupResult Prev(*this, Name, D.getIdentifierLoc(), LookupOrdinaryName,
+                          ForRedeclaration);
+        LookupQualifiedName(Prev, DC);
+        assert(!Prev.isAmbiguous() &&
+               "Cannot have an ambiguity in previous-declaration lookup");
+        for (LookupResult::iterator Func = Prev.begin(), FuncEnd = Prev.end();
+             Func != FuncEnd; ++Func) {
+          if (isa<FunctionDecl>(*Func) &&
+              isNearlyMatchingFunction(Context, cast<FunctionDecl>(*Func), NewFD))
+            Diag((*Func)->getLocation(), diag::note_member_def_close_match);
+        }
+      }
+    } else if (!isFriend) { 
+      // The user provided a superfluous scope specifier inside a class definition:
       //
       // class X {
-      //   void f() const;
+      //   void X::f();
       // };
-      //
-      // void X::f() { } // ill-formed
-      //
-      // Complain about this problem, and attempt to suggest close
-      // matches (e.g., those that differ only in cv-qualifiers and
-      // whether the parameter types are references).
-      Diag(D.getIdentifierLoc(), diag::err_member_def_does_not_match)
-        << Name << DC << D.getCXXScopeSpec().getRange();
-      NewFD->setInvalidDecl();
-
-      LookupResult Prev(*this, Name, D.getIdentifierLoc(), LookupOrdinaryName,
-                        ForRedeclaration);
-      LookupQualifiedName(Prev, DC);
-      assert(!Prev.isAmbiguous() &&
-             "Cannot have an ambiguity in previous-declaration lookup");
-      for (LookupResult::iterator Func = Prev.begin(), FuncEnd = Prev.end();
-           Func != FuncEnd; ++Func) {
-        if (isa<FunctionDecl>(*Func) &&
-            isNearlyMatchingFunction(Context, cast<FunctionDecl>(*Func), NewFD))
-          Diag((*Func)->getLocation(), diag::note_member_def_close_match);
-      }
+      Diag(NewFD->getLocation(), diag::warn_member_extra_qualification)
+        << Name << FixItHint::CreateRemoval(D.getCXXScopeSpec().getRange());
     }
   }
 
diff --git a/test/FixIt/fixit.cpp b/test/FixIt/fixit.cpp
index 9b39c9e8a5..95d8a88af1 100644
--- a/test/FixIt/fixit.cpp
+++ b/test/FixIt/fixit.cpp
@@ -66,3 +66,8 @@ CT<1> main(void); // expected-error{{'main' must return 'int'}}
 // typedef CT<1> mainT(void);
 // mainT main; // TODO
 
+// extra qualification on member
+class C {
+  int C::foo();
+};
+
diff --git a/test/SemaCXX/class.cpp b/test/SemaCXX/class.cpp
index ad0792c9c4..7c6a62f41e 100644
--- a/test/SemaCXX/class.cpp
+++ b/test/SemaCXX/class.cpp
@@ -117,7 +117,7 @@ struct C4 {
 struct S
 {
   void f(); // expected-note 1 {{previous declaration}}
-  void S::f() {} // expected-error {{class member cannot be redeclared}} expected-note {{previous declaration}} expected-note {{previous definition}}
+  void S::f() {} // expected-warning {{extra qualification on member}} expected-error {{class member cannot be redeclared}} expected-note {{previous declaration}} expected-note {{previous definition}}
   void f() {} // expected-error {{class member cannot be redeclared}} expected-error {{redefinition}}
 };
 
diff --git a/test/SemaCXX/nested-name-spec.cpp b/test/SemaCXX/nested-name-spec.cpp
index 0dc1097e38..bef8570bff 100644
--- a/test/SemaCXX/nested-name-spec.cpp
+++ b/test/SemaCXX/nested-name-spec.cpp
@@ -244,3 +244,7 @@ namespace PR7133 {
     return false;
   }
 }
+
+class CLASS {
+  void CLASS::foo2(); // expected-warning {{extra qualification on member 'foo2'}}
+};