On Tue, 2018-11-20 at 21:57 +0100, Jakub Jelinek wrote: > Hi! > > This PR is complaining about range covering the first token from > an id-expression: > pr87386.C:4:15: error: static assertion failed: foo > 4 | static_assert(foo::test<int>::value, "foo"); > | ^~~ > The following patch adjust that to: > pr87386.C:4:31: error: static assertion failed: foo > 4 | static_assert(foo::test<int>::value, "foo"); > | ~~~~~~~~~~~~~~~~^~~~~ > instead, though as the changes to the testsuite show, not really sure > if it is a good idea in all cases, because then we sometimes print: > ... bar is not in foo namespace, did you mean 'baz' > foo::bar > ~~~~~^~~ > baz > where the baz is misaligned.
Is the compiler suggesting the use of (a) "foo::baz" or (b) "::baz"? Given the underlining, the fix-it hint would be suggesting the replacement of "foo::bar" with "baz", which would be wrong if we mean (a) above. (c.f. "Fix-it hints should work" in https://gcc.gnu.org/onlinedocs/gcci nt/Guidelines-for-Diagnostics.html ) FWIW in r265610 I ran into issues like this, which I resolved by holding off on some fix-it hints for the case where we don't have a location covering the whole of a qualified name. > Would it be better to just print > pr87386.C:4:31: error: static assertion failed: foo > 4 | static_assert(foo::test<int>::value, "foo"); > | ^~~~~ > instead? That would mean dropping the cp_parser_id_expression change > and readjusting or dropping some testsuite changes. That might be better... let me look at the affected test cases. [...snip...] > /* Parse a template-declaration. > --- gcc/testsuite/g++.dg/spellcheck-pr79298.C.jj 2018-10-31 > 10:31:13.281572644 +0100 > +++ gcc/testsuite/g++.dg/spellcheck-pr79298.C 2018-11-20 > 19:14:19.208219955 +0100 > @@ -11,7 +11,7 @@ int foo () > return M::y; // { dg-error ".y. is not a member of .M." } > /* { dg-begin-multiline-output "" } > return M::y; > - ^ > + ~~~^ > { dg-end-multiline-output "" } */ > } > @@ -20,7 +20,7 @@ int bar () > return O::colour; // { dg-error ".colour. is not a member of .O.; > did you mean 'color'\\?" } > /* { dg-begin-multiline-output "" } > return O::colour; > - ^~~~~~ > - color > + ~~~^~~~~~ > + color > { dg-end-multiline-output "" } */ > } This makes the fix-it hint wrong: after the fix-it is applied, it will become return color; (which won't compile), rather than return O::color; which will. (I wish we had a good automated way of verifying that fix-it hints fix things) > --- gcc/testsuite/g++.dg/lookup/suggestions2.C.jj 2018-10-31 > 10:31:06.928677642 +0100 > +++ gcc/testsuite/g++.dg/lookup/suggestions2.C 2018-11-20 > 19:12:05.281395810 +0100 > @@ -33,8 +33,8 @@ int test_1_long (void) { > return outer_ns::var_in_inner_ns_a; // { dg-error "did you mean > 'var_in_outer_ns'" } > /* { dg-begin-multiline-output "" } > return outer_ns::var_in_inner_ns_a; > - ^~~~~~~~~~~~~~~~~ > - var_in_outer_ns > + ~~~~~~~~~~^~~~~~~~~~~~~~~~~ > + var_in_outer_ns > { dg-end-multiline-output "" } */ > } Again, this makes the fix-it hint wrong: after the fix-it is applied, it will become return var_in_outer_ns; (which won't compile) rather than: return outer_ns::var_in_outer_ns; [...snip; I think there are more examples in this test file...] > --- gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C.jj 20 > 18-10-31 10:31:07.765663807 +0100 > +++ gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C 2018- > 11-20 19:14:35.698952024 +0100 > @@ -73,7 +73,7 @@ void test_3 () > ns3::goo_3 (); // { dg-error "'goo_3' is not a member of 'ns3'; > did you mean 'foo_3'\\?" } > /* { dg-begin-multiline-output "" } > ns3::goo_3 (); > - ^~~~~ > - foo_3 > + ~~~~~^~~~~ > + foo_3 > { dg-end-multiline-output "" } */ > } Again, the fix-it hint becomes wrong, it will become: foo_3 (); rather than: ns3::foo_3 (); [...snip...] > --- gcc/testsuite/g++.dg/spellcheck-pr77829.C.jj 2018-10-31 > 10:31:10.213623350 +0100 > +++ gcc/testsuite/g++.dg/spellcheck-pr77829.C 2018-11-20 > 19:13:30.700008045 +0100 > @@ -21,8 +21,8 @@ void fn_1_explicit () > detail::some_type i; // { dg-error ".some_type. is not a member of > .detail.; did you mean 'some_typedef'\\?" } > /* { dg-begin-multiline-output "" } > detail::some_type i; > - ^~~~~~~~~ > - some_typedef > + ~~~~~~~~^~~~~~~~~ > + some_typedef > { dg-end-multiline-output "" } */ > } Similar problems here. [...snip...] So it looks like the less invasive fix might be better (not that I've looked at it in detail, though). Hope this is constructive Dave
