On 12/10/24 6:05 PM, David Malcolm wrote:
On Sat, 2024-12-07 at 10:19 -0500, Jason Merrill wrote:
On 11/15/24 8:02 PM, David Malcolm wrote:
This patch is a followup to:
"c++: use diagnostic nesting [PR116253]"
Following Sy Brand's UX suggestions in P2429R0 for example 1, this
patch
tweaks print_z_candidates to add a note about the number of
candidates,
and adds a candidate number to each one.
Various examples of output can be seen in the testsuite part of the
patch.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
* call.cc (print_z_candidates): Count the number of
candidates and issue a note stating the count at an
intermediate nesting level. Number the individual
candidates.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic9.C: Update expected
results for candidate count and numbering.
* g++.dg/concepts/nested-diagnostics-1-truncated.C:
* g++.dg/concepts/nested-diagnostics-1.C: Likewise.
* g++.dg/concepts/nested-diagnostics-2.C: Likewise.
* g++.dg/cpp23/explicit-obj-lambda11.C: Likewise.
* g++.dg/cpp2a/desig4.C: Likewise.
* g++.dg/cpp2a/desig6.C: Likewise.
* g++.dg/cpp2a/spaceship-eq15.C: Likewise.
* g++.dg/diagnostic/function-color1.C: Likewise.
* g++.dg/diagnostic/param-type-mismatch-2.C: Likewise.
* g++.dg/diagnostic/pr100716-1.C: Likewise.
* g++.dg/diagnostic/pr100716.C: Likewise.
* g++.dg/lookup/operator-2.C: Likewise.
* g++.dg/lookup/pr80891-5.C: Likewise.
* g++.dg/modules/adhoc-1_b.C: Likewise.
* g++.dg/modules/err-1_c.C: Likewise.
* g++.dg/modules/err-1_d.C: Likewise.
* g++.dg/other/return2.C: Likewise.
* g++.dg/overload/error6.C: Likewise.
* g++.dg/template/local6.C: Likewise.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
---
gcc/cp/call.cc | 16 ++++++-
gcc/testsuite/g++.dg/concepts/diagnostic9.C | 2 +-
.../concepts/nested-diagnostics-1-truncated.C | 25 +++++------
.../g++.dg/concepts/nested-diagnostics-1.C | 43 ++++++++++----
-----
.../g++.dg/concepts/nested-diagnostics-2.C | 27 ++++++------
.../g++.dg/cpp23/explicit-obj-lambda11.C | 3 +-
gcc/testsuite/g++.dg/cpp2a/desig4.C | 4 +-
gcc/testsuite/g++.dg/cpp2a/desig6.C | 4 +-
gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C | 8 ++--
.../g++.dg/diagnostic/function-color1.C | 2 +-
.../g++.dg/diagnostic/param-type-mismatch-2.C | 10 ++++-
gcc/testsuite/g++.dg/diagnostic/pr100716-1.C | 14 +++---
gcc/testsuite/g++.dg/diagnostic/pr100716.C | 14 +++---
gcc/testsuite/g++.dg/lookup/operator-2.C | 2 +-
gcc/testsuite/g++.dg/lookup/pr80891-5.C | 6 +--
gcc/testsuite/g++.dg/modules/adhoc-1_b.C | 4 +-
gcc/testsuite/g++.dg/modules/err-1_c.C | 10 ++---
gcc/testsuite/g++.dg/modules/err-1_d.C | 6 +--
gcc/testsuite/g++.dg/other/return2.C | 2 +-
gcc/testsuite/g++.dg/overload/error6.C | 4 +-
gcc/testsuite/g++.dg/template/local6.C | 2 +-
21 files changed, 116 insertions(+), 92 deletions(-)
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index db9eb1a55cfc..f820419ee4ff 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -4134,6 +4134,16 @@ print_z_candidates (location_t loc, struct
z_candidate *candidates,
auto_diagnostic_nesting_level sentinel;
+ int num_candidates = 0;
+ for (auto iter = candidates; iter; iter = iter->next)
+ ++num_candidates;
+
+ inform_n (loc,
+ num_candidates, "we found %i candidate", "we found %i
candidates",
The use of 'we' here is novel, especially since we're talking about
objective properties of the TU. I'd say "there are".
FWIW I actually prefer the wording "I found" (like does Elm).
Flow's style guide:
https://calebmer.com/2019/07/01/writing-good-compiler-error-messages.html
says "Write messages in first person plural. That is, use “we”. For
example “we see an error”. This personifies the compiler as a team of
people looking for bugs in the developer’s code. By personifying our
type checker error messages feel like a dialogue. Elm’s error messages
are famous for using first person: “I see an error”. First person feels
a bit uncomfortable to me. A compiler is certainly not a single person,
nor is it built by a single person. I prefer “we” as a compromise."
Sy Brand suggests "we found" in P2429R0, where they have in their
notes: "Rephrased the error in terms of "we can't find a matching
function" rather than "no matching function"."
As you note, we're talking about objective properties of the TU, and
using "we" is a novel wording.
Given that, I'll change it to "there are" (impersonal present tense),
but I wonder if this approach always works if we're also offering
suggestions. For example, if, say, I forget a * or & on a param at a
callsite, I'd prefer the compiler focus on the suggestion for what I
probably meant to type rather perhaps numerous rejected candidates for
what I actually typed (if that makes sense). But maybe that's just too
hard to implement given C++'s complexity...
Where possible, absolutely. I like the "did you mean" diagnostics and
general friendliness, it just feels weird to me for the compiler to
refer to itself in the first person (singular or plural); if we decided
to start doing that there should be a wider discussion and it should be
recorded in doc/ux.texi.
Do we want this line at all for the case of 1 candidate?
In P2429R0 example 1 has a single candidate, and Sy Brand's proposed
design shows the line for that case, with:
"We found 1 candidate:"
so I'm inclined to say "yes".
Fair enough, OK with the "there are" change.
Jason
