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... > > 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". Thanks Dave
