[Clang] Emit a diagnostic note at the class declaration when the method definition does not match any declaration. #110638

c8ef · 2024-10-01T09:09:09Z

In this patch, we will emit a diagnostic note pointing to the class declaration when a method definition does not match any declaration. This approach, similar to what GCC does, makes the diagnostic more user-friendly.

llvmbot · 2024-10-01T12:59:55Z

@llvm/pr-subscribers-clang

Author: None (c8ef)

Changes

Fixes #110558.

In this patch, we will emit a diagnostic note pointing to the class declaration when a method definition does not match any declaration. This approach, similar to what GCC does, makes the diagnostic more user-friendly.

Patch is 22.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110638.diff

24 Files Affected:

(modified) clang/lib/Sema/SemaDecl.cpp (+8-3)
(modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp (+1)
(modified) clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p8.cpp (+1)
(modified) clang/test/CXX/dcl/dcl.fct/p17.cpp (+1)
(modified) clang/test/CXX/drs/cwg22xx.cpp (+1)
(modified) clang/test/CXX/drs/cwg3xx.cpp (+6-1)
(modified) clang/test/CXX/special/class.inhctor/p8.cpp (+1)
(modified) clang/test/CXX/temp/temp.constr/temp.constr.decl/func-template-decl.cpp (+1)
(modified) clang/test/CXX/temp/temp.res/temp.local/p8.cpp (+4-4)
(modified) clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp (+2)
(modified) clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp (+1)
(modified) clang/test/CXX/temp/temp.spec/temp.expl.spec/p18.cpp (+1)
(modified) clang/test/FixIt/member-mismatch.cpp (+2)
(modified) clang/test/Parser/cxx-class.cpp (+1)
(modified) clang/test/SemaCXX/attr-target-mv.cpp (+4-3)
(modified) clang/test/SemaCXX/attr-target-version.cpp (+4-3)
(modified) clang/test/SemaCXX/enable_if.cpp (+1-1)
(modified) clang/test/SemaCXX/function-redecl.cpp (+8-6)
(modified) clang/test/SemaCXX/lambda-unevaluated.cpp (+2)
(modified) clang/test/SemaCXX/nested-name-spec.cpp (+4)
(modified) clang/test/SemaCXX/out-of-line-def-mismatch.cpp (+5-1)
(modified) clang/test/SemaCXX/typo-correction.cpp (+1)
(modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+3-1)
(modified) clang/test/SemaTemplate/recovery-crash.cpp (+3-2)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0e536f71a2f70d..e4c45cbc09e0f7 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9086,9 +9086,14 @@ static NamedDecl *DiagnoseInvalidRedeclaration(
   SemaRef.Diag(NewFD->getLocation(), DiagMsg)
       << Name << NewDC << IsDefinition << NewFD->getLocation();
 
-  bool NewFDisConst = false;
-  if (CXXMethodDecl *NewMD = dyn_cast<CXXMethodDecl>(NewFD))
-    NewFDisConst = NewMD->isConst();
+  CXXMethodDecl *NewMD = dyn_cast<CXXMethodDecl>(NewFD);
+  if (NewMD && DiagMsg == diag::err_member_decl_does_not_match) {
+    CXXRecordDecl *RD = NewMD->getParent();
+    SemaRef.Diag(RD->getLocation(), diag::note_defined_here)
+        << RD->getName() << RD->getLocation();
+  }
+
+  bool NewFDisConst = NewMD && NewMD->isConst();
 
   for (SmallVectorImpl<std::pair<FunctionDecl *, unsigned> >::iterator
        NearMatch = NearMatches.begin(), NearMatchEnd = NearMatches.end();
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
index 9e890204c78bd6..f0901733f8afeb 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
@@ -170,5 +170,6 @@ namespace ImplicitConstexprDef {
 
   constexpr void A::f() { } // expected-warning {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const' to avoid a change in behavior}}
                             // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'ImplicitConstexprDef::A'}}
+                            // expected-note@-6 {{A defined here}}
 }
 #endif
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p8.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p8.cpp
index 0454412229fad7..fbd26cfd7b9db7 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p8.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p8.cpp
@@ -6,3 +6,4 @@ void A::f(enum { e2 }) {} // expected-error{{cannot be defined in a parameter}}
 
 enum { e3 } A::g() { } // expected-error{{cannot be defined in the result type}} \
 // expected-error{{out-of-line definition}}
+// expected-note@-6{{defined here}}
diff --git a/clang/test/CXX/dcl/dcl.fct/p17.cpp b/clang/test/CXX/dcl/dcl.fct/p17.cpp
index d7487233f5d5c2..34b0c9f3ce0273 100644
--- a/clang/test/CXX/dcl/dcl.fct/p17.cpp
+++ b/clang/test/CXX/dcl/dcl.fct/p17.cpp
@@ -110,6 +110,7 @@ namespace unconstrained {
   template<typename U>
   constexpr auto S<T>::f2(auto x, U u, T t) -> decltype(x + u + t) { return x + u + t; }
   // expected-error@-1 {{out-of-line definition of 'f2' does not match any declaration in 'S<T>'}}
+  // expected-note@-14 {{S defined here}}
 
   template<typename T>
   template<typename U>
diff --git a/clang/test/CXX/drs/cwg22xx.cpp b/clang/test/CXX/drs/cwg22xx.cpp
index 797c3ed8546ef1..a16a74a43a859a 100644
--- a/clang/test/CXX/drs/cwg22xx.cpp
+++ b/clang/test/CXX/drs/cwg22xx.cpp
@@ -108,6 +108,7 @@ namespace MultilevelSpecialization {
   template<> template<int a, int b>
     void B<int, int>::f(int i, int (&arr1)[a], int (&arr2)[b]) {}
     // since-cxx11-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg2233::MultilevelSpecialization::B<int, int>'}}
+    // expected-note@-6 {{defined here}}
   template<> template<>
     void B<int, int>::f<1, 1>(int i, int (&arr1a)[1], int (&arr2a)[1]) {}
 }
diff --git a/clang/test/CXX/drs/cwg3xx.cpp b/clang/test/CXX/drs/cwg3xx.cpp
index 7157feed3f7626..1ef878dab4e56a 100644
--- a/clang/test/CXX/drs/cwg3xx.cpp
+++ b/clang/test/CXX/drs/cwg3xx.cpp
@@ -600,6 +600,7 @@ namespace cwg336 { // cwg336: yes
     template<> template<class X> class A<int>::B {};
     template<> template<> template<class T> void A<int>::B<double>::mf1(T t) {}
     // expected-error@-1 {{out-of-line definition of 'mf1' does not match any declaration in 'cwg336::Pre::A<int>::B<double>'}}
+    // expected-note@-3 {{defined here}}
     template<class Y> template<> void A<Y>::B<double>::mf2() {}
     // expected-error@-1 {{nested name specifier 'A<Y>::B<double>::' for declaration does not refer into a class, class template or class template partial specialization}}
   }
@@ -766,8 +767,10 @@ namespace cwg347 { // cwg347: yes
   // expected-error@-1 {{no member named 'n' in 'cwg347::derived'}}
   void derived::f() {}
   // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg347::derived'}}
+  // expected-note@-8 {{defined here}}
   void derived::g() {}
   // expected-error@-1 {{out-of-line definition of 'g' does not match any declaration in 'cwg347::derived'}}
+  // expected-note@-11 {{defined here}}
 }
 
 // cwg348: na
@@ -1014,13 +1017,15 @@ namespace cwg357 { // cwg357: yes
   };
   template<typename T> void A<T>::f() {}
   // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'A<T>'}}
-  //   expected-note@#cwg357-f {{member declaration does not match because it is const qualified}}
+  // expected-note@-5 {{defined here}}
+  // expected-note@#cwg357-f {{member declaration does not match because it is const qualified}}
 
   struct B {
     template<typename T> void f();
   };
   template<typename T> void B::f() const {}
   // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg357::B'}}
+  // expected-note@-5 {{defined here}}
 }
 
 namespace cwg358 { // cwg358: yes
diff --git a/clang/test/CXX/special/class.inhctor/p8.cpp b/clang/test/CXX/special/class.inhctor/p8.cpp
index 58c01d2b912d4a..fe17d6e44351f4 100644
--- a/clang/test/CXX/special/class.inhctor/p8.cpp
+++ b/clang/test/CXX/special/class.inhctor/p8.cpp
@@ -30,3 +30,4 @@ struct D : C {
 static_assert(D(123).v == 123, "");
 
 template<typename T> constexpr D::D(T t) : C(t) {} // expected-error {{does not match any declaration in 'D'}}
+                                                   // expected-note@-6 {{defined here}}
diff --git a/clang/test/CXX/temp/temp.constr/temp.constr.decl/func-template-decl.cpp b/clang/test/CXX/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
index 30fbec64eea782..e511404ffac38c 100644
--- a/clang/test/CXX/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
+++ b/clang/test/CXX/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
@@ -55,5 +55,6 @@ struct TA {
 template <unsigned N>
 template <template <unsigned> class TT> int TA<N>::A() { return sizeof(TT<N>); }
 // expected-error@-1{{out-of-line definition of 'A' does not match any declaration in 'TA<N>'}}
+// expected-note@-8{{defined here}}
 
 } // end namespace diag
diff --git a/clang/test/CXX/temp/temp.res/temp.local/p8.cpp b/clang/test/CXX/temp/temp.res/temp.local/p8.cpp
index 56599985da06e9..4cc48e16699a6b 100644
--- a/clang/test/CXX/temp/temp.res/temp.local/p8.cpp
+++ b/clang/test/CXX/temp/temp.res/temp.local/p8.cpp
@@ -127,19 +127,19 @@ namespace SearchClassBetweenTemplateParameterLists {
   template<typename T> template<typename BB>
   void A<T>::B<BB>::g(BB) { // expected-error {{does not match}}
     BB bb; // expected-error {{incomplete type}}
-  }
+  } // expected-note@-58 {{defined here}}
 
   // error, 'AA' found in (4)
   template<typename AA> template<typename U>
   void A<AA>::B<U>::h(AA) { // expected-error {{does not match}}
     AA aa; // expected-error {{incomplete type}}
-  }
+  } // expected-note@-64 {{defined here}}
 
   // error, 'BB' found in (2)
   template<typename BB> template<typename U>
   void A<BB>::B<U>::i(BB) { // expected-error {{does not match}}
     BB bb; // expected-error {{incomplete type}}
-  }
+  } // expected-note@-70 {{defined here}}
 
   // OK, 'BB' found in (1)
   template<typename T> template<typename U> template<typename BB>
@@ -151,7 +151,7 @@ namespace SearchClassBetweenTemplateParameterLists {
   template<typename T> template<typename BB> template<typename V>
   void A<T>::B<BB>::k(V) { // expected-error {{does not match}}
     BB bb; // expected-error {{incomplete type}}
-  }
+  } // expected-note@-82 {{defined here}}
 
   int CC;
   template <typename> struct C;
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp
index 2a574890836956..bd36ea2318d761 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p12.cpp
@@ -49,11 +49,13 @@ template<>
 constexpr void B<short>::g0(); // since-cxx14-error {{constexpr declaration of 'g0' follows non-constexpr declaration}}
                                // cxx11-error@-1 {{out-of-line declaration of 'g0' does not match any declaration in 'B<short>'}}
                                // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+                               // expected-note@-18 {{defined here}}
 
 template<>
 constexpr void B<short>::g1(); // since-cxx14-error {{out-of-line declaration of 'g1' does not match any declaration in 'B<short>'}}
                                // cxx11-error@-1 {{constexpr declaration of 'g1' follows non-constexpr declaration}}
                                // cxx11-warning@-2 {{'constexpr' non-static member function will not be implicitly 'const' in C++14; add 'const'}}
+                               // expected-note@-24 {{defined here}}
 
 template<>
 template<typename U>
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
index a023bf84137d78..cf800dce3a37f5 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p14-23.cpp
@@ -56,6 +56,7 @@ namespace N0 {
 
   template<>
   constexpr int A<0>::h() { return 2; } // expected-error {{out-of-line definition of 'h' does not match any declaration in 'N0::A<0>'}}
+                                        // expected-note@-48 {{defined here}}
 
   static_assert(A<5>::h() == 0);
   static_assert(A<4>::h() == 1);
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p18.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p18.cpp
index 4d175a8860870d..390a47c8942690 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p18.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p18.cpp
@@ -14,6 +14,7 @@ template<> template<> template<class T>
 
 template<> template<> template<class T>
 void A<long>::B<double>::mf1(T t) { } // expected-error{{does not match}}
+                                      // expected-note@-7{{defined here}}
 
 // FIXME: This diagnostic could probably be better.
 template<class Y> template<>
diff --git a/clang/test/FixIt/member-mismatch.cpp b/clang/test/FixIt/member-mismatch.cpp
index 2d2aec2e176b2b..531184933c09ab 100644
--- a/clang/test/FixIt/member-mismatch.cpp
+++ b/clang/test/FixIt/member-mismatch.cpp
@@ -8,5 +8,7 @@ class Foo {
 
 // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:15-[[@LINE+1]]:15}:" const"
 int Foo::get() {} // expected-error {{does not match any declaration}}
+                  // expected-note@-7 {{defined here}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:20-[[@LINE+1]]:26}:""
 void Foo::set(int) const {} // expected-error {{does not match any declaration}}
+                            // expected-note@-10 {{defined here}}
diff --git a/clang/test/Parser/cxx-class.cpp b/clang/test/Parser/cxx-class.cpp
index c90c7e030a8bd5..583534afa83408 100644
--- a/clang/test/Parser/cxx-class.cpp
+++ b/clang/test/Parser/cxx-class.cpp
@@ -103,6 +103,7 @@ namespace ctor_error {
   class Foo {};
   // By [class.qual]p2, this is a constructor declaration.
   Foo::Foo (F) = F(); // expected-error{{does not match any declaration in 'ctor_error::Foo'}}
+                      // expected-note@-3{{defined here}}
 
   class Ctor { // expected-note{{not complete until the closing '}'}}
     Ctor(f)(int); // ok
diff --git a/clang/test/SemaCXX/attr-target-mv.cpp b/clang/test/SemaCXX/attr-target-mv.cpp
index 5b2f0fc825f399..004ac0469492fb 100644
--- a/clang/test/SemaCXX/attr-target-mv.cpp
+++ b/clang/test/SemaCXX/attr-target-mv.cpp
@@ -187,7 +187,8 @@ struct BadOutOfLine {
 
 int __attribute__((target("sse4.2"))) BadOutOfLine::foo(int) { return 0; }
 int __attribute__((target("default"))) BadOutOfLine::foo(int) { return 1; }
-// expected-error@+3 {{out-of-line definition of 'foo' does not match any declaration in 'BadOutOfLine'}}
-// expected-note@-3 {{member declaration nearly matches}}
-// expected-note@-3 {{member declaration nearly matches}}
+// expected-error@+4 {{out-of-line definition of 'foo' does not match any declaration in 'BadOutOfLine'}}
+// expected-note@-8 {{defined here}}
+// expected-note@-4 {{member declaration nearly matches}}
+// expected-note@-4 {{member declaration nearly matches}}
 int __attribute__((target("arch=atom"))) BadOutOfLine::foo(int) { return 1; }
diff --git a/clang/test/SemaCXX/attr-target-version.cpp b/clang/test/SemaCXX/attr-target-version.cpp
index b3385f043590f8..c8a05e99c80ddd 100644
--- a/clang/test/SemaCXX/attr-target-version.cpp
+++ b/clang/test/SemaCXX/attr-target-version.cpp
@@ -103,7 +103,8 @@ class Out {
 };
 int __attribute__((target_version("bti"))) Out::func(void) { return 1; }
 int __attribute__((target_version("ssbs2"))) Out::func(void) { return 2; }
-// expected-error@+3 {{out-of-line definition of 'func' does not match any declaration in 'Out'}}
-// expected-note@-3 {{member declaration nearly matches}}
-// expected-note@-3 {{member declaration nearly matches}}
+// expected-error@+4 {{out-of-line definition of 'func' does not match any declaration in 'Out'}}
+// expected-note@-2 {{member declaration nearly matches}}
+// expected-note@-4 {{member declaration nearly matches}}
+// expected-note@-9 {{defined here}}
 int __attribute__((target_version("rng"))) Out::func(void) { return 3; }
diff --git a/clang/test/SemaCXX/enable_if.cpp b/clang/test/SemaCXX/enable_if.cpp
index 5eec02a643340a..1c307881e5d4a5 100644
--- a/clang/test/SemaCXX/enable_if.cpp
+++ b/clang/test/SemaCXX/enable_if.cpp
@@ -5,7 +5,7 @@ int surrogate(int);
 struct Incomplete;  // expected-note{{forward declaration of 'Incomplete'}} \
                     // expected-note {{forward declaration of 'Incomplete'}}
 
-struct X {
+struct X { // expected-note{{defined here}}
   X() = default;  // expected-note{{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
   X(const X&) = default;  // expected-note{{candidate constructor not viable: no known conversion from 'bool' to 'const X' for 1st argument}}
   X(bool b) __attribute__((enable_if(b, "chosen when 'b' is true")));  // expected-note{{candidate disabled: chosen when 'b' is true}}
diff --git a/clang/test/SemaCXX/function-redecl.cpp b/clang/test/SemaCXX/function-redecl.cpp
index 34d2acc75430c3..8c0e9a880d070e 100644
--- a/clang/test/SemaCXX/function-redecl.cpp
+++ b/clang/test/SemaCXX/function-redecl.cpp
@@ -61,7 +61,7 @@ void B::Notypocorrection(int) { // expected-error {{out-of-line definition of 'N
 }
 
 struct X { int f(); };
-struct Y : public X {};
+struct Y : public X {}; // expected-note {{defined here}}
 int Y::f() { return 3; } // expected-error {{out-of-line definition of 'f' does not match any declaration in 'Y'}}
 
 namespace test1 {
@@ -70,7 +70,7 @@ struct Foo {
 };
 }
 
-class Bar {
+class Bar { // expected-note {{defined here}}
   void f(test1::Foo::Inner foo) const; // expected-note {{member declaration does not match because it is const qualified}}
 };
 
@@ -80,7 +80,8 @@ void Bar::f(Foo::Inner foo) { // expected-error {{out-of-line definition of 'f'
   (void)foo;
 }
 
-class Crash {
+class Crash { // expected-note {{defined here}}
+              // expected-note@-1 {{defined here}}
  public:
   void GetCart(int count) const;
 };
@@ -89,7 +90,8 @@ void Crash::cart(int count) const {} // expected-error {{out-of-line definition
 // ...while this one crashed clang
 void Crash::chart(int count) const {} // expected-error {{out-of-line definition of 'chart' does not match any declaration in 'Crash'}}
 
-class TestConst {
+class TestConst { // expected-note {{defined here}}
+                  // expected-note@-1 {{defined here}}
  public:
   int getit() const; // expected-note {{member declaration does not match because it is const qualified}}
   void setit(int); // expected-note {{member declaration does not match because it is not const qualified}}
@@ -102,7 +104,7 @@ int TestConst::getit() { // expected-error {{out-of-line definition of 'getit' d
 void TestConst::setit(int) const { // expected-error {{out-of-line definition of 'setit' does not match any declaration in 'TestConst'}}
 }
 
-struct J { int typo() const; };
+struct J { int typo() const; }; // expected-note {{defined here}}
 int J::typo_() { return 3; } // expected-error {{out-of-line definition of 'typo_' does not match any declaration in 'J'}}
 
 // Ensure we correct the redecl of Foo::isGood to Bar::Foo::isGood and not
@@ -126,7 +128,7 @@ bool Foo::isGood() { // expected-error {{out-of-line definition of 'isGood' does
 void Foo::beEvil() {} // expected-error {{out-of-line definition of 'beEvil' does not match any declaration in namespace 'redecl_typo::Foo'; did you mean 'BeEvil'?}}
 }
 
-struct CVQualFun {
+struct CVQualFun { // expected-note {{defined here}}
   void func(int a, int &b); // expected-note {{type of 2nd parameter of member declaration does not match definition ('int &' vs 'int')}}
 };
 
diff --git a/clang/test/SemaCXX/lambda-unevaluated.cpp b/clang/test/SemaCXX/lambda-unevaluated.cpp
index 39ee89bc797f84..f0ebfef029d16e 100644
--- a/clang/test/SemaCXX/lambda-unevaluated.cpp
+++ b/clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -76,6 +76,7 @@ struct A {
 
 template <class T>
 void A<T>::spam(decltype([] {})) // expected-error{{out-of-line definition of 'spam' does not match}}
+                                 // expected-note@-6{{defined here}}
 {}
 
 struct B {
@@ -85,6 +86,7 @@ struct B {
 
 template <class T>
 void B::spam(decltype([] {})) {} // expected-error{{out-of-line definition of 'spam' does not match}}
+                                 // expected-note@-7{{defined here}}
 
 } // namespace GH51416
 
diff --git a/clang/test/SemaCXX/nested-name-spec.cpp b/clang/test/SemaCXX/nested-name-spec.cpp
index 920ef42bc15646..cd66792f4852b5 100644
--- a/clang/test/SemaCXX/nested-name-spec.cpp
+++ b/clang/test/SemaCXX/nested-name-spec.cpp
@@ -36,8 +36,10 @@ class C2 {
 };
 
 void C2::m() const { } // expected-error{{out-of-line definition of 'm' does not match any declaration in 'C2'}}
+                       // expected-note@-11{{defined here}}
 
 void C2::f(int) { } // expected-error{{out-of-line definition of 'f' does not match any declaration in 'C2'}}
+                    // expected-note@-14{{defined here}}
 
 void C2::m() {
   x = 0;
@@ -128,6 +130,7 @@ class Operators {
 };
 
 Operators Operators::operator+(const Operators&) { // expected-error{{out-of-line definition of 'operator+' does not match any declaration in 'Operators'}}
+                                                   // expected-note@-6{{defined here}}
   Operators ops;
   return ops;
 }
@@ -152,6 +155,7 @@ void A::g(const int&) { } // expected-error{{out-of-line definition of 'g' does
 struct Struct { };
 
 void Struct::f() { } // expected-error{{out-of-line definition of 'f' does not match any declaration in 'Struct'}}
+                     // expected-note@-3{{defined here}}
 
 void global_func(int);
 void global_func2(int);
diff --git a/clang/test/SemaCXX/out-of-line-def-mismatch.cpp b/clang/test/SemaCXX/out-of-line-def-mismatch.cpp
index a4e130d234353a..1ce1cc0c7dfc0b 100644
--- a/clang/test/SemaCXX/out-of-line-def-mismatch.cpp
+++ b/clang/test/SemaCXX/out-of-line-def-mismatch.cpp
@@ -6,7 +6,11 @@ namespace N2 {
   namespace N1 {
     class C1 {};
 
-    struct S2 {
+    struct S2 { // expected-note {{defined here}}
+                // expected-note@-1 {{defined here}}
+                // expected-note@-2 {{defined here}}
+                // expected-note@-3 {{defined here}}
+                // expected-note@-4 {{defined here}}
       void func(S1*); // expected-note {{type of 1st parameter of member declaration does not match definition ('S1 *' (aka 'N2::S1 *') vs 'S1 *' (aka 'N2::N1::S1 *'))}}
       void func(C1&, unsigned, const S1*); // expected-note {{type of 3rd parameter of member declaration does not match definition ('const S1 *' (aka 'const N2::...
[truncated]

c8ef · 2024-10-01T13:00:23Z

CC @hokein @shafik @Sirraide

Endilll · 2024-10-01T13:37:33Z

clang/test/CXX/drs/cwg22xx.cpp

@@ -108,6 +108,7 @@ namespace MultilevelSpecialization {
  template<> template<int a, int b>
    void B<int, int>::f(int i, int (&arr1)[a], int (&arr2)[b]) {}
    // since-cxx11-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg2233::MultilevelSpecialization::B<int, int>'}}
+    // expected-note@-6 {{defined here}}


Suggested change

// expected-note@-6 {{defined here}}

// expected-note@#cwg2233-blah {{defined here}}

We shouldn't make the readers count lines to understand, which line the diagnostic is emitted for. Use bookmarks instead (with a proper name, see other tests for the reference).

Got it. I'll update the test case. Does the implementation itself look good? I just want to make sure since many test cases are affected.

Yeah, adding a diagnostic that’s very general like this one tends to require updating a lot of tests.

Done. The current test cases now use bookmarks to pinpoint class declarations.

Endilll · 2024-10-01T13:38:08Z

clang/test/CXX/drs/cwg3xx.cpp

@@ -600,6 +600,7 @@ namespace cwg336 { // cwg336: yes
    template<> template<class X> class A<int>::B {};
    template<> template<> template<class T> void A<int>::B<double>::mf1(T t) {}
    // expected-error@-1 {{out-of-line definition of 'mf1' does not match any declaration in 'cwg336::Pre::A<int>::B<double>'}}
+    // expected-note@-3 {{defined here}}


Suggested change

// expected-note@-3 {{defined here}}

// expected-note@#cwg366-blah {{defined here}}

Endilll

Implementation seems fine to me, but wait for other reviewers before merging.

hokein

Thanks, the change looks good to me.

Please also add a note in the clang/docs/ReleaseNotes.rst

Sirraide · 2024-10-01T19:23:15Z

clang/test/CXX/drs/cwg22xx.cpp

@@ -108,6 +108,7 @@ namespace MultilevelSpecialization {
  template<> template<int a, int b>
    void B<int, int>::f(int i, int (&arr1)[a], int (&arr2)[b]) {}
    // since-cxx11-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg2233::MultilevelSpecialization::B<int, int>'}}
+    // expected-note@-6 {{defined here}}


Yeah, adding a diagnostic that’s very general like this one tends to require updating a lot of tests.

c8ef · 2024-10-02T05:22:42Z

Please also add a note in the clang/docs/ReleaseNotes.rst

Done.

Endilll · 2024-10-02T09:15:59Z

clang/test/CXX/drs/cwg3xx.cpp

@@ -597,9 +597,10 @@ namespace cwg336 { // cwg336: yes
        void mf2();
      };
    };
-    template<> template<class X> class A<int>::B {};
+    template<> template<class X> class A<int>::B {}; // #defined-here-cwg336


Suggested change

template<> template<class X> class A<int>::B {}; // #defined-here-cwg336

template<> template<class X> class A<int>::B {}; // #cwg336-B

Endilll · 2024-10-02T09:16:16Z

clang/test/CXX/drs/cwg3xx.cpp

    template<> template<> template<class T> void A<int>::B<double>::mf1(T t) {}
    // expected-error@-1 {{out-of-line definition of 'mf1' does not match any declaration in 'cwg336::Pre::A<int>::B<double>'}}
+    // expected-note@#defined-here-cwg336 {{defined here}}


Suggested change

// expected-note@#defined-here-cwg336 {{defined here}}

// expected-note@#cwg336-B {{defined here}}

Endilll · 2024-10-02T09:16:33Z

clang/test/CXX/drs/cwg3xx.cpp

@@ -758,16 +759,18 @@ namespace cwg347 { // cwg347: yes
    void g();
  };

-  struct derived : base {};
+  struct derived : base {}; // #defined-here-derived


Suggested change

struct derived : base {}; // #defined-here-derived

struct derived : base {}; // #cwg347-derived

Endilll · 2024-10-02T09:16:47Z

clang/test/CXX/drs/cwg3xx.cpp


  struct derived::nested {};
  // expected-error@-1 {{no struct named 'nested' in 'cwg347::derived'}}
  int derived::n;
  // expected-error@-1 {{no member named 'n' in 'cwg347::derived'}}
  void derived::f() {}
  // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg347::derived'}}
+  // expected-note@#defined-here-derived {{defined here}}


Suggested change

// expected-note@#defined-here-derived {{defined here}}

// expected-note@#cwg347-derived {{defined here}}

Endilll · 2024-10-02T09:17:03Z

clang/test/CXX/drs/cwg3xx.cpp

  void derived::g() {}
  // expected-error@-1 {{out-of-line definition of 'g' does not match any declaration in 'cwg347::derived'}}
+  // expected-note@#defined-here-derived {{defined here}}


Suggested change

// expected-note@#defined-here-derived {{defined here}}

// expected-note@#cwg347-derived {{defined here}}

Endilll · 2024-10-02T09:17:49Z

clang/test/CXX/drs/cwg3xx.cpp

    void f() const; // #cwg357-f
  };
  template<typename T> void A<T>::f() {}
  // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'A<T>'}}
-  //   expected-note@#cwg357-f {{member declaration does not match because it is const qualified}}
+  // expected-note@#defined-here-cwg357-A {{defined here}}


Suggested change

// expected-note@#defined-here-cwg357-A {{defined here}}

// expected-note@#cwg357-A {{defined here}}

Endilll · 2024-10-02T09:18:14Z

clang/test/CXX/drs/cwg3xx.cpp


-  struct B {
+  struct B { // #defined-here-cwg357-B


Suggested change

struct B { // #defined-here-cwg357-B

struct B { // #cwg357-B

Endilll · 2024-10-02T09:18:25Z

clang/test/CXX/drs/cwg3xx.cpp

    template<typename T> void f();
  };
  template<typename T> void B::f() const {}
  // expected-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg357::B'}}
+  // expected-note@#defined-here-cwg357-B {{defined here}}


Suggested change

// expected-note@#defined-here-cwg357-B {{defined here}}

// expected-note@#cwg357-B {{defined here}}

Endilll · 2024-10-02T09:19:07Z

clang/test/CXX/drs/cwg22xx.cpp

@@ -102,12 +102,13 @@ namespace MultilevelSpecialization {
  // default argument -- how far back do we look when determining whether a
  // parameter was expanded from a pack?
  //   -- zygoloid 2020-06-02
-  template<typename ...T> struct B {
+  template<typename ...T> struct B { // #defined-here


Suggested change

template<typename ...T> struct B { // #defined-here

template<typename ...T> struct B { // #cwg2233-B

Endilll · 2024-10-02T09:19:18Z

clang/test/CXX/drs/cwg22xx.cpp

    template <T... V> void f(int i = 0, int (&... arr)[V]);
  };
  template<> template<int a, int b>
    void B<int, int>::f(int i, int (&arr1)[a], int (&arr2)[b]) {}
    // since-cxx11-error@-1 {{out-of-line definition of 'f' does not match any declaration in 'cwg2233::MultilevelSpecialization::B<int, int>'}}
+    // expected-note@#defined-here {{defined here}}


Suggested change

// expected-note@#defined-here {{defined here}}

// since-cxx11-note@#cwg2233-B {{defined here}}

Endilll

LGTM otherwise

clang/test/CXX/drs/cwg3xx.cpp

Co-authored-by: Vlad Serebrennikov <[email protected]>

hokein · 2024-10-02T14:39:35Z

Thanks for your work on this. The buildbot is green, so I’ll merge it now.

c8ef · 2024-10-02T14:40:59Z

Thanks for all your thorough review and guidance!

…od definition does not match any declaration. (llvm#110638) Fixes llvm#110558. In this patch, we will emit a diagnostic note pointing to the class declaration when a method definition does not match any declaration. This approach, similar to what GCC does, makes the diagnostic more user-friendly. --------- Co-authored-by: Vlad Serebrennikov <[email protected]>

c8ef added 4 commits October 1, 2024 09:08

110558

38430b6

fix

8f37ce5

fix test

c875c6c

fix test

e20b2e6

c8ef changed the title ~~Draft~~ [Clang] Emit a diagnostic note at the class declaration when the method definition does not match any declaration. Oct 1, 2024

c8ef marked this pull request as ready for review October 1, 2024 12:59

c8ef requested a review from Endilll as a code owner October 1, 2024 12:59

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 1, 2024

Endilll reviewed Oct 1, 2024

View reviewed changes

hokein approved these changes Oct 1, 2024

View reviewed changes

Sirraide approved these changes Oct 1, 2024

View reviewed changes

c8ef added 2 commits October 2, 2024 10:51

Merge branch 'main' into diag-class-name

79e06ba

release note

2fc1074

mizvekov approved these changes Oct 2, 2024

View reviewed changes

c8ef added 2 commits October 2, 2024 03:22

fix test

e1bff7a

fix test

8cf3db7

Endilll reviewed Oct 2, 2024

View reviewed changes

c8ef added 2 commits October 2, 2024 09:53

Address review comments.

716d31c

Merge branch 'main' into diag-class-name

98afb33

Endilll reviewed Oct 2, 2024

View reviewed changes

clang/test/CXX/drs/cwg3xx.cpp Outdated Show resolved Hide resolved

Update clang/test/CXX/drs/cwg3xx.cpp

5f69f6f

Co-authored-by: Vlad Serebrennikov <[email protected]>

hokein merged commit 8282c58 into llvm:main Oct 2, 2024
9 checks passed

c8ef deleted the diag-class-name branch October 2, 2024 14:41

	// expected-note@-6 {{defined here}}
	// expected-note@#cwg2233-blah {{defined here}}

	// expected-note@-3 {{defined here}}
	// expected-note@#cwg366-blah {{defined here}}

	template<> template<class X> class A<int>::B {}; // #defined-here-cwg336
	template<> template<class X> class A<int>::B {}; // #cwg336-B

	// expected-note@#defined-here-cwg336 {{defined here}}
	// expected-note@#cwg336-B {{defined here}}

	struct derived : base {}; // #defined-here-derived
	struct derived : base {}; // #cwg347-derived

	// expected-note@#defined-here-derived {{defined here}}
	// expected-note@#cwg347-derived {{defined here}}

	// expected-note@#defined-here-cwg357-A {{defined here}}
	// expected-note@#cwg357-A {{defined here}}

	// expected-note@#defined-here-cwg357-B {{defined here}}
	// expected-note@#cwg357-B {{defined here}}

	template<typename ...T> struct B { // #defined-here
	template<typename ...T> struct B { // #cwg2233-B

	// expected-note@#defined-here {{defined here}}
	// since-cxx11-note@#cwg2233-B {{defined here}}

[Clang] Emit a diagnostic note at the class declaration when the method definition does not match any declaration. #110638

[Clang] Emit a diagnostic note at the class declaration when the method definition does not match any declaration. #110638

Uh oh!

Conversation

c8ef commented Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 1, 2024

Uh oh!

c8ef commented Oct 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Endilll left a comment

Choose a reason for hiding this comment

Uh oh!

hokein left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c8ef commented Oct 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Endilll left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hokein commented Oct 2, 2024

Uh oh!

Uh oh!

c8ef commented Oct 2, 2024

Uh oh!

Uh oh!

c8ef commented Oct 1, 2024 •

edited

Loading