On 4/24/24 13:22, Patrick Palka wrote:
Tested on x86_64-pc-linux-gnu, full bootstrap+regtest in progress, does this look OK if successful?-- >8 -- It seems the diagnostic machinery's source line printing respects the pretty printer prefix, but this is undesirable for the call to diagnostic_show_locus in print_instantiation_partial_context_line added in r14-4388-g1c45319b66edc9 since the prefix may have been set when issuing an earlier, unrelated diagnostic and we just want to print an unprefixed source line. This patch naively fixes this by clearing the prefix before calling diagnostic_show_locus. Before this patch, for error60a.C below we'd print gcc/testsuite/g++.dg/template/error60a.C: In function ‘void usage()’: gcc/testsuite/g++.dg/template/error60a.C:24:3: error: ‘unrelated_error’ was not declared in this scope 24 | unrelated_error; // { dg-error "not declared" } | ^~~~~~~~~~~~~~~ gcc/testsuite/g++.dg/template/error60a.C: In instantiation of ‘void test(Foo) [with Foo = int]’: gcc/testsuite/g++.dg/template/error60a.C:25:13: required from here gcc/testsuite/g++.dg/template/error60a.C:24:3: error: 25 | test<int> (42); // { dg-message " required from here" } gcc/testsuite/g++.dg/template/error60a.C:24:3: error: | ~~~~~~~~~~^~~~ gcc/testsuite/g++.dg/template/error60a.C:19:24: error: invalid conversion from ‘int’ to ‘int*’ [-fpermissive] 19 | my_pointer<Foo> ptr (val); // { dg-error "invalid conversion from 'int' to 'int\\*'" } | ^~~ | | | int gcc/testsuite/g++.dg/template/error60a.C:9:20: note: initializing argument 1 of ‘my_pointer<Foo>::my_pointer(Foo*) [with Foo = int]’ 9 | my_pointer (Foo *ptr) // { dg-message " initializing argument 1" } | ~~~~~^~~ and afterward we print gcc/testsuite/g++.dg/template/error60a.C: In function ‘void usage()’: gcc/testsuite/g++.dg/template/error60a.C:24:3: error: ‘unrelated_error’ was not declared in this scope 24 | unrelated_error; // { dg-error "not declared" } | ^~~~~~~~~~~~~~~ gcc/testsuite/g++.dg/template/error60a.C: In instantiation of ‘void test(Foo) [with Foo = int]’: gcc/testsuite/g++.dg/template/error60a.C:25:13: required from here 25 | test<int> (42); // { dg-message " required from here" } | ~~~~~~~~~~^~~~ gcc/testsuite/g++.dg/template/error60a.C:19:24: error: invalid conversion from ‘int’ to ‘int*’ [-fpermissive] 19 | my_pointer<Foo> ptr (val); // { dg-error "invalid conversion from 'int' to 'int\\*'" } | ^~~ | | | int gcc/testsuite/g++.dg/template/error60a.C:9:20: note: initializing argument 1 of ‘my_pointer<Foo>::my_pointer(Foo*) [with Foo = int]’ 9 | my_pointer (Foo *ptr) // { dg-message " initializing argument 1" } | ~~~~~^~~ gcc/cp/ChangeLog: * error.cc (print_instantiation_partial_context_line): Clear context->printer->prefix around the call to diagnostic_show_locus. gcc/testsuite/ChangeLog: * g++.dg/concepts/diagnostic2.C: Expect source line printed for the required from here message. * g++.dg/template/error60a.C: New test. --- gcc/cp/error.cc | 2 + gcc/testsuite/g++.dg/concepts/diagnostic2.C | 6 ++- gcc/testsuite/g++.dg/template/error60a.C | 46 +++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/template/error60a.C diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc index 7074845154e..a7067d4d2ed 100644 --- a/gcc/cp/error.cc +++ b/gcc/cp/error.cc @@ -3793,7 +3793,9 @@ print_instantiation_partial_context_line (diagnostic_context *context, : _("required from here\n")); } gcc_rich_location rich_loc (loc); + char *saved_prefix = pp_take_prefix (context->printer); diagnostic_show_locus (context, &rich_loc, DK_NOTE); + context->printer->prefix = saved_prefix;
I would follow the pattern of c_diagnostic_finalizer here, i.e. using pp_set_prefix for the restore.
Though I think the pp_set_prefix to NULL in c_diagnostic_finalizer is redundant and should have been removed in r9-2240-g653fee1961981f when the previous line changed from _set_ to _take_. If it isn't redundant, then it should be, i.e. pp_take_prefix should call it instead of directly setting NULL.
Some _take_ callers do set(NULL) and others don't; they should definitely be consistent one way or the other.
David, what do you think? Jason
