On Tue, 2024-12-10 at 19:08 -0500, Jason Merrill wrote: > 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.
Thanks; I've pushed with the updated wording as r15-6116- gd3dd24acd74605 (after bootstrap®rtesting on x86_64-pc-linux-gnu), and the followup as r15-6117-g7435d1dbae8ae1. Dave
