This patch improves the UX for various cases of "no declaration matches"
where print_candidates encounters a close match.

For example, consider the const vs non-const here:

class foo
{
public:
  void test (int i, int j, void *ptr, int k);
};

// Wrong "const"-ness of param 3.
void foo::test (int i, int j, const void *ptr, int k)
{
}

where we emit (with indentation provided by the prior patch):

test.cc:8:6: error: no declaration matches ‘void foo::test(int, int, const 
void*, int)’
    8 | void foo::test (int i, int j, const void *ptr, int k)
      |      ^~~
  • there is 1 candidate
    • candidate is: ‘void foo::test(int, int, void*, int)’
      test.cc:4:8:
          4 |   void test (int i, int j, void *ptr, int k);
            |        ^~~~
test.cc:1:7: note: ‘class foo’ defined here
    1 | class foo
      |       ^~~

which requires the user to look through the pairs of parameters
and try to find the mismatch by eye.

This patch adds notes identifying that parameter 3 has the mismatch, and
what the mismatch is, using a pair of colors to highlight and contrast
the type mismatch.

test.cc:8:6: error: no declaration matches ‘void foo::test(int, int, const 
void*, int)’
    8 | void foo::test (int i, int j, const void *ptr, int k)
      |      ^~~
  • there is 1 candidate
    • candidate is: ‘void foo::test(int, int, void*, int)’
      test.cc:4:8:
          4 |   void test (int i, int j, void *ptr, int k);
            |        ^~~~
      • parameter 3 of candidate has type ‘void*’...
        test.cc:4:34:
            4 |   void test (int i, int j, void *ptr, int k);
              |                            ~~~~~~^~~
      • ...which does not match type ‘const void*’
        test.cc:8:43:
            8 | void foo::test (int i, int j, const void *ptr, int k)
              |                               ~~~~~~~~~~~~^~~
test.cc:1:7: note: ‘class foo’ defined here
    1 | class foo
      |       ^~~

This also works for the "this" case, improving the UX for messing up
const vs non-const between decls and defns of member functions; see
bad-fndef-2.C for an example.

For screenshots showing the colorization effect, see slides 9-12 of my
Cauldron talk:
https://gcc.gnu.org/wiki/cauldron2025#What.27s_new_with_diagnostics_in_GCC_16
("Hierarchical diagnostics (not yet in trunk)").

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?
Dave

gcc/cp/ChangeLog:
        * call.cc (get_fndecl_argument_location): Use DECL_SOURCE_LOCATION
        for "this".
        * cp-tree.h (class candidate_context): New.
        (print_candidates): Add optional candidate_context param.
        * decl2.cc: Define INCLUDE_VECTOR.  Include "gcc-rich-location.h"
        and "tree-pretty-print-markup.h".
        (struct fndecl_signature): New.
        (class parm_rich_location): New.
        (class fndecl_comparison): New.
        (check_classfn): For the "no declaration matches" case, pass an
        instance of a custom candidate_context subclass to
        print_candidates, using fndecl_comparison to report on close
        matches.
        * pt.cc (print_candidates): Add optional candidate_context param.
        Use it if provided to potentially emit per-candidate notes.

gcc/testsuite/ChangeLog:
        * g++.dg/diagnostic/bad-fndef-1.C: Add directives to expect
        "void *" vs "const void *" notes about parameter 3 of the close
        candidate.
        * g++.dg/diagnostic/bad-fndef-2.C: New test.
        * g++.dg/diagnostic/bad-fndef-3.C: New test.
        * g++.dg/diagnostic/bad-fndef-4.C: New test.
        * g++.dg/diagnostic/bad-fndef-5.C: New test.
        * g++.dg/diagnostic/bad-fndef-6.C: New test.

Signed-off-by: David Malcolm <[email protected]>
---
 gcc/cp/call.cc                                |   3 +
 gcc/cp/cp-tree.h                              |  14 +-
 gcc/cp/decl2.cc                               | 192 +++++++++++++++++-
 gcc/cp/pt.cc                                  |  17 +-
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-1.C |   2 +
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-2.C |  15 ++
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-3.C |  16 ++
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-4.C |  38 ++++
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-5.C |  15 ++
 gcc/testsuite/g++.dg/diagnostic/bad-fndef-6.C |  17 ++
 10 files changed, 323 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/bad-fndef-2.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/bad-fndef-3.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/bad-fndef-4.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/bad-fndef-5.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/bad-fndef-6.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index fc4e6da1f9a20..ab37530a42917 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -8593,6 +8593,9 @@ get_fndecl_argument_location (tree fndecl, int argnum)
   if (DECL_ARTIFICIAL (fndecl))
     return DECL_SOURCE_LOCATION (fndecl);
 
+  if (argnum == -1)
+    return DECL_SOURCE_LOCATION (fndecl);
+
   int i;
   tree param;
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index d0d057c927017..f5fc8bab6eaeb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7891,7 +7891,19 @@ extern tree most_specialized_instantiation       (tree);
 extern tree most_specialized_partial_spec       (tree, tsubst_flags_t, bool = 
false);
 extern tree most_constrained_function          (tree);
 extern void inform_num_candidates              (location_t, int);
-extern void print_candidates                   (tree);
+
+/* Abstract base class for optionally providing extra diagnostic note(s)
+   about a candidate in calls to print_candidates.  */
+
+class candidate_context
+{
+public:
+  virtual ~candidate_context () {}
+  virtual void emit_any_notes_for_candidate (tree cand_fndecl) = 0;
+};
+extern void print_candidates                   (tree,
+                                                candidate_context * = nullptr);
+
 extern void instantiate_pending_templates      (int);
 extern tree tsubst_default_argument            (tree, int, tree, tree,
                                                 tsubst_flags_t);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 9e135af41b340..1a123c8cda311 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
    line numbers.  For example, the CONST_DECLs for enum values.  */
 
 #include "config.h"
+#define INCLUDE_VECTOR
 #include "system.h"
 #include "coretypes.h"
 #include "memmodel.h"
@@ -53,6 +54,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-inline.h"
 #include "escaped_string.h"
 #include "contracts.h"
+#include "gcc-rich-location.h"
+#include "tree-pretty-print-markup.h"
 
 /* Id for dumping the raw trees.  */
 int raw_dump_id;
@@ -770,6 +773,170 @@ check_member_template (tree tmpl)
     error ("template declaration of %q#D", decl);
 }
 
+/* A struct for conveniently working with the parameter types of a
+   fndecl.  */
+
+struct fndecl_signature
+{
+  fndecl_signature (tree fndecl)
+  : m_fndecl (fndecl),
+    m_num_artificial_parms (num_artificial_parms_for (fndecl))
+  {
+    function_args_iterator iter;
+    tree argtype;
+    FOREACH_FUNCTION_ARGS (TREE_TYPE (fndecl), argtype, iter)
+      m_parm_types.safe_push (argtype);
+    gcc_assert (m_parm_types.length () >= (size_t)m_num_artificial_parms);
+    m_num_user_parms = m_parm_types.length () - m_num_artificial_parms;
+  }
+
+  /* Convert from index within DECL_ARGUMENTS to 0-based user-written
+     parameter, or -1 for "this".  */
+  int
+  get_argno_from_idx (size_t idx) const
+  {
+    return static_cast<int> (idx) - m_num_artificial_parms;
+  }
+
+  location_t
+  get_argno_location (int argno) const
+  {
+    return get_fndecl_argument_location (m_fndecl, argno);
+  }
+
+  tree m_fndecl;
+  int m_num_artificial_parms;
+  auto_vec<tree> m_parm_types;
+  size_t m_num_user_parms;
+};
+
+/* A rich_location subclass that highlights a given argno
+   within FNDECL_SIG using HIGHLIGHT_COLOR in the quoted source.  */
+
+class parm_rich_location : public gcc_rich_location
+{
+public:
+  parm_rich_location (const fndecl_signature &fndecl_sig,
+                     int argno,
+                     const char *highlight_color)
+  : gcc_rich_location (fndecl_sig.get_argno_location (argno),
+                      nullptr,
+                      highlight_color)
+  {
+  }
+};
+
+/* A class for comparing a pair of fndecls and emitting diagnostic notes
+   about the differences between them, if they are sufficiently close.  */
+
+class fndecl_comparison
+{
+public:
+  fndecl_comparison (tree decl_a,
+                    tree decl_b)
+  : m_decl_a (decl_a),
+    m_decl_b (decl_b)
+  {
+    gcc_assert (TREE_CODE (decl_a) == FUNCTION_DECL);
+    gcc_assert (TREE_CODE (decl_b) == FUNCTION_DECL);
+
+  }
+
+  /* Attempt to emit diagnostic notes describing the differences between
+     the fndecls, when they are a sufficiently close match.  */
+  void
+  maybe_emit_notes_for_close_match () const
+  {
+    std::vector<parm_difference> differences = get_differences ();
+    if (differences.size () == 1)
+      {
+       auto_diagnostic_nesting_level sentinel;
+       print_parm_type_difference (differences[0]);
+      }
+  }
+
+private:
+  struct parm_difference
+  {
+    size_t m_parm_idx_a;
+    size_t m_parm_idx_b;
+  };
+
+  /* If we have a close match, return the differences between the two
+     fndecls.  Otherwise return an empty vector.  */
+  std::vector<parm_difference>
+  get_differences () const
+  {
+    std::vector<parm_difference> differences;
+
+    /* For now, just handle the case where the "user parm" count is
+       equal.  */
+    if (m_decl_a.m_num_user_parms != m_decl_b.m_num_user_parms)
+      return differences;
+
+    /* Ideally we'd check the edit distance, and thus report on close
+       matches which are missing a param, have transposed params, etc.
+       For now, just iterate through the params, finding differences
+       elementwise.  */
+
+    /* Find differences in artificial params, if they are comparable.
+       This should find e.g. const vs non-const differences in "this".  */
+    if (m_decl_a.m_num_artificial_parms == m_decl_b.m_num_artificial_parms)
+      for (int parm_idx = 0;
+          parm_idx < m_decl_a.m_num_artificial_parms;
+          ++parm_idx)
+       compare (parm_idx, parm_idx, differences);
+
+    // Find differences in user-provided params:
+    for (size_t user_parm_idx = 0;
+        user_parm_idx < m_decl_a.m_num_user_parms;
+        ++user_parm_idx)
+      {
+       const size_t idx_a = m_decl_a.m_num_artificial_parms + user_parm_idx;
+       const size_t idx_b = m_decl_b.m_num_artificial_parms + user_parm_idx;
+       compare (idx_a, idx_b, differences);
+      }
+
+    return differences;
+  }
+
+  void
+  compare (size_t idx_a, size_t idx_b,
+          std::vector<parm_difference> &differences) const
+  {
+    if (m_decl_a.m_parm_types[idx_a]
+       != m_decl_b.m_parm_types[idx_b])
+      differences.push_back (parm_difference {idx_a, idx_b});
+  }
+
+  void
+  print_parm_type_difference (const parm_difference &diff) const
+  {
+    const char * const highlight_a = "highlight-a";
+    pp_markup::element_quoted_type type_of_parm_a
+      (m_decl_a.m_parm_types[diff.m_parm_idx_a],
+       highlight_a);
+    const int argno_a = m_decl_a.get_argno_from_idx (diff.m_parm_idx_a);
+    parm_rich_location rich_loc_a (m_decl_a, argno_a, highlight_a);
+    inform (&rich_loc_a,
+           "parameter %P of candidate has type %e...",
+           argno_a, &type_of_parm_a);
+
+    const char * const highlight_b = "highlight-b";
+    pp_markup::element_quoted_type type_of_parm_b
+      (m_decl_b.m_parm_types[diff.m_parm_idx_b],
+       highlight_b);
+    const int argno_b = m_decl_b.get_argno_from_idx (diff.m_parm_idx_b);
+    parm_rich_location rich_loc_b (m_decl_b, argno_b, highlight_b);
+    inform (&rich_loc_b,
+           "...which does not match type %e",
+           &type_of_parm_b);
+  }
+
+  fndecl_signature m_decl_a;
+  fndecl_signature m_decl_b;
+};
+
 /* Sanity check: report error if this function FUNCTION is not
    really a member of the class (CTYPE) it is supposed to belong to.
    TEMPLATE_PARMS is used to specify the template parameters of a member
@@ -917,7 +1084,30 @@ check_classfn (tree ctype, tree function, tree 
template_parms)
          error_at (DECL_SOURCE_LOCATION (function),
                    "no declaration matches %q#D", function);
          if (fns)
-           print_candidates (fns);
+           {
+             class decl_mismatch_context : public candidate_context
+             {
+             public:
+               decl_mismatch_context (tree function)
+               : m_function (function)
+               {
+                 gcc_assert (TREE_CODE (function) == FUNCTION_DECL);
+               }
+
+               void
+               emit_any_notes_for_candidate (tree cand) final override
+               {
+                 if (TREE_CODE (cand) == FUNCTION_DECL)
+                   {
+                     fndecl_comparison diff (cand, m_function);
+                     diff.maybe_emit_notes_for_close_match ();
+                   }
+               }
+             private:
+               tree m_function;
+             } ctxt (function);
+             print_candidates (fns, &ctxt);
+           }
          else if (DECL_CONV_FN_P (function))
            inform (DECL_SOURCE_LOCATION (function),
                    "no conversion operators declared");
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 59e9585aac94d..9a94ddf7ccf44 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -2060,10 +2060,13 @@ inform_num_candidates (location_t loc, int 
num_candidates)
 }
 
 /* Print the list of candidate FNS in an error message.  FNS can also
-   be a TREE_LIST of non-functions in the case of an ambiguous lookup.  */
+   be a TREE_LIST of non-functions in the case of an ambiguous lookup.
+
+   If CAND_CTXT is non-null, use it for each candidate to allow for
+   additional per-candidate notes.  */
 
 void
-print_candidates (tree fns)
+print_candidates (tree fns, candidate_context *cand_ctxt)
 {
   auto_vec<tree> candidates;
   flatten_candidates (fns, candidates);
@@ -2078,13 +2081,19 @@ print_candidates (tree fns)
     {
       tree cand = candidates[0];
       inform (DECL_SOURCE_LOCATION (cand), "candidate is: %#qD", cand);
+      if (cand_ctxt)
+       cand_ctxt->emit_any_notes_for_candidate (cand);
     }
   else
     {
       int idx = 0;
       for (auto cand : candidates)
-       inform (DECL_SOURCE_LOCATION (cand), "candidate %i: %#qD",
-               ++idx, cand);
+       {
+         inform (DECL_SOURCE_LOCATION (cand), "candidate %i: %#qD",
+                 ++idx, cand);
+         if (cand_ctxt)
+           cand_ctxt->emit_any_notes_for_candidate (cand);
+       }
     }
 }
 
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-1.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-1.C
index 1156072b11f16..60d6c5baa0bb9 100644
--- a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-1.C
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-1.C
@@ -13,3 +13,5 @@ void foo::test (int i, int j, const void *ptr, int k) // { 
dg-line defn }
 // { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
 // { dg-message "8: candidate 1: " "candidate 1" { target *-*-* } other_decl }
 // { dg-message "8: candidate 2: " "candidate 2" { target *-*-* } close_decl }
+// { dg-message "34: parameter 3 of candidate has type 'void\\*'" "param of 
decl" { target *-*-* } close_decl }
+// { dg-message "43: \\.\\.\\.which does not match type 'const void\\*'" 
"param of defn" { target *-*-* } defn }
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-2.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-2.C
new file mode 100644
index 0000000000000..068b392928dc0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-2.C
@@ -0,0 +1,15 @@
+class foo // { dg-message "'class foo' defined here" }
+{
+public:
+  void test (int i, int j, int k); // { dg-line decl }
+};
+
+// Wrong "const"-ness of this:
+void foo::test (int i, int j, int k) const // { dg-line defn }
+{
+}
+
+// { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
+// { dg-message "8: candidate is: " "candidate is" { target *-*-* } decl }
+// { dg-message "8: parameter 'this' of candidate has type 'foo\\*'" "this of 
decl" { target *-*-* } decl }
+// { dg-message "6: \\.\\.\\.which does not match type 'const foo\\*'" "this 
of defn" { target *-*-* } defn }
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-3.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-3.C
new file mode 100644
index 0000000000000..452334dc5728c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-3.C
@@ -0,0 +1,16 @@
+class foo // { dg-message "'class foo' defined here" }
+{
+public:
+  void test (int i, int j, int k) const; // { dg-line decl }
+};
+
+// Wrong "const"-ness of "this":
+
+void foo::test (int i, int j, int k) // { dg-line defn }
+{
+}
+
+// { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
+// { dg-message "8: candidate is: " "candidate is" { target *-*-* } decl }
+// { dg-message "8: parameter 'this' of candidate has type 'const foo\\*'" 
"this of decl" { target *-*-* } decl }
+// { dg-message "6: \\.\\.\\.which does not match type 'foo\\*'" "this of 
defn" { target *-*-* } defn }
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-4.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-4.C
new file mode 100644
index 0000000000000..afbcb76cb28af
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-4.C
@@ -0,0 +1,38 @@
+// { dg-additional-options "-fdiagnostics-show-caret" }
+
+class foo // { dg-message "'class foo' defined here" }
+{
+public:
+  void test (int i, int j, void *ptr, int k); // { dg-line decl }
+};
+
+// Wrong "const"-ness of a param (param 3).
+void foo::test (int i, int j, const void *ptr, int k) // { dg-line defn }
+{
+}
+
+// { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
+/* { dg-begin-multiline-output "" }
+ void foo::test (int i, int j, const void *ptr, int k)
+      ^~~
+   { dg-end-multiline-output "" } */
+// { dg-message "8: candidate is: " "candidate is" { target *-*-* } decl }
+/* { dg-begin-multiline-output "" }
+   void test (int i, int j, void *ptr, int k);
+        ^~~~
+   { dg-end-multiline-output "" } */
+// { dg-message "34: parameter 3 of candidate has type 'void\\*'" "param of 
decl" { target *-*-* } decl }
+/* { dg-begin-multiline-output "" }
+   void test (int i, int j, void *ptr, int k);
+                            ~~~~~~^~~
+   { dg-end-multiline-output "" } */
+// { dg-message "43: \\.\\.\\.which does not match type 'const void\\*'" 
"param of defn" { target *-*-* } defn }
+/* { dg-begin-multiline-output "" }
+ void foo::test (int i, int j, const void *ptr, int k)
+                               ~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ class foo
+       ^~~
+   { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-5.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-5.C
new file mode 100644
index 0000000000000..498395f4e1525
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-5.C
@@ -0,0 +1,15 @@
+class foo // { dg-message "'class foo' defined here" }
+{
+public:
+  static void test (int i, int j, void *ptr, int k); // { dg-line decl }
+};
+
+// Wrong "const"-ness of a param, for static member fn (param 3).
+void foo::test (int i, int j, const void *ptr, int k) // { dg-line defn }
+{
+}
+
+// { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
+// { dg-message "15: candidate is: " "candidate is" { target *-*-* } decl }
+// { dg-message "41: parameter 3 of candidate has type 'void\\*'" "param of 
decl" { target *-*-* } decl }
+// { dg-message "43: \\.\\.\\.which does not match type 'const void\\*'" 
"param of defn" { target *-*-* } defn }
diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-fndef-6.C 
b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-6.C
new file mode 100644
index 0000000000000..5a34395180fa5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/bad-fndef-6.C
@@ -0,0 +1,17 @@
+class bar;
+
+class foo // { dg-message "'class foo' defined here" }
+{
+public:
+  void test (int i, foo *j, int k); // { dg-line decl }
+};
+
+// Missing '*' on a param (param 2).
+void foo::test (int i, foo j, int k) // { dg-line defn }
+{
+}
+
+// { dg-error "6: no declaration matches" "error" { target *-*-* } defn }
+// { dg-message "8: candidate is: " "candidate is" { target *-*-* } decl }
+// { dg-message "26: parameter 2 of candidate has type 'foo\\*'" "param of 
decl" { target *-*-* } decl }
+// { dg-message "28: \\.\\.\\.which does not match type 'foo'" "param of defn" 
{ target *-*-* } defn }
-- 
2.26.3

Reply via email to