| Message ID | 66c1e508.050a0220.31958c.0679@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: Slightly clean up error for referencing TU-local entity | expand |
On 8/18/24 8:11 AM, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > Or should we even just remove the warning entirely? I'm not sure it > really adds all that much, since it's usual AFAICT for errors to prevent > the intended outputs from being generated. Agreed, removing it seems fine. > -- >8 -- > > It was pointed out to me that the current error referencing an internal > linkage entity reads almost like an ICE message, with the message > finishing with the unhelpful: > > m.cpp:1:8: error: failed to write compiled module: Bad file data > 1 | export module M; > | ^~~~~~ > > It would probably be clearer to just emit the same warning that we do in > other cases where we don't write modules due to errors. > > gcc/cp/ChangeLog: > > * module.cc (module_state::write_begin): Return a boolean to > indicate errors rather than just doing to->set_error(). > (finish_module_processing): Check for failed write_begin and > disable module writing in that case. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/block-decl-2.C: Adjust error message. > * g++.dg/modules/internal-1.C: Likewise. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/module.cc | 30 ++++++++++++--------- > gcc/testsuite/g++.dg/modules/block-decl-2.C | 2 +- > gcc/testsuite/g++.dg/modules/internal-1.C | 2 +- > 3 files changed, 19 insertions(+), 15 deletions(-) > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index f4d137b13a1..9f23feece09 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -3681,7 +3681,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { > > public: > /* Read and write module. */ > - void write_begin (elf_out *to, cpp_reader *, > + bool write_begin (elf_out *to, cpp_reader *, > module_state_config &, unsigned &crc); > void write_end (elf_out *to, cpp_reader *, > module_state_config &, unsigned &crc); > @@ -18317,7 +18317,7 @@ ool_cmp (const void *a_, const void *b_) > MOD_SNAME_PFX.cfg : config data > */ > > -void > +bool > module_state::write_begin (elf_out *to, cpp_reader *reader, > module_state_config &config, unsigned &crc) > { > @@ -18395,10 +18395,7 @@ module_state::write_begin (elf_out *to, cpp_reader *reader, > table.find_dependencies (this); > > if (!table.finalize_dependencies ()) > - { > - to->set_error (); > - return; > - } > + return false; > > #if CHECKING_P > /* We're done verifying at-most once reading, reset to verify > @@ -18595,6 +18592,8 @@ module_state::write_begin (elf_out *to, cpp_reader *reader, > // so-controlled. > if (false) > write_env (to); > + > + return true; > } > > // Finish module writing after we've emitted all dynamic initializers. > @@ -20847,22 +20846,27 @@ finish_module_processing (cpp_reader *reader) > > cookie = new module_processing_cookie (cmi_name, tmp_name, fd, e); > > + bool report_error = false; > if (errorcount > /* Don't write the module if it contains an erroneous template. */ > || (erroneous_templates > && !erroneous_templates->is_empty ())) > - warning_at (state->loc, 0, "not writing module %qs due to errors", > - state->get_flatname ()); > + report_error = true; > else if (cookie->out.begin ()) > { > - cookie->began = true; > - auto loc = input_location; > /* So crashes finger-point the module decl. */ > - input_location = state->loc; > - state->write_begin (&cookie->out, reader, cookie->config, cookie->crc); > - input_location = loc; > + iloc_sentinel ils = state->loc; > + if (state->write_begin (&cookie->out, reader, cookie->config, > + cookie->crc)) > + cookie->began = true; > + else > + report_error = true; > } > > + if (report_error) > + warning_at (state->loc, 0, "not writing module %qs due to errors", > + state->get_flatname ()); > + > dump.pop (n); > timevar_stop (TV_MODULE_EXPORT); > > diff --git a/gcc/testsuite/g++.dg/modules/block-decl-2.C b/gcc/testsuite/g++.dg/modules/block-decl-2.C > index 974e26f9b7a..90f18b30945 100644 > --- a/gcc/testsuite/g++.dg/modules/block-decl-2.C > +++ b/gcc/testsuite/g++.dg/modules/block-decl-2.C > @@ -18,4 +18,4 @@ export extern "C++" auto foo() { > return X{}; > } > > -// { dg-prune-output "failed to write compiled module" } > +// { dg-prune-output "not writing module" } > diff --git a/gcc/testsuite/g++.dg/modules/internal-1.C b/gcc/testsuite/g++.dg/modules/internal-1.C > index 45d3bf06f28..399dd68b92e 100644 > --- a/gcc/testsuite/g++.dg/modules/internal-1.C > +++ b/gcc/testsuite/g++.dg/modules/internal-1.C > @@ -1,6 +1,6 @@ > // { dg-additional-options "-fmodules-ts" } > > -export module frob; // { dg-error "failed to write" } > +export module frob; // { dg-warning "not writing module" } > // { dg-module-cmi !frob } > > namespace {
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index f4d137b13a1..9f23feece09 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -3681,7 +3681,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { public: /* Read and write module. */ - void write_begin (elf_out *to, cpp_reader *, + bool write_begin (elf_out *to, cpp_reader *, module_state_config &, unsigned &crc); void write_end (elf_out *to, cpp_reader *, module_state_config &, unsigned &crc); @@ -18317,7 +18317,7 @@ ool_cmp (const void *a_, const void *b_) MOD_SNAME_PFX.cfg : config data */ -void +bool module_state::write_begin (elf_out *to, cpp_reader *reader, module_state_config &config, unsigned &crc) { @@ -18395,10 +18395,7 @@ module_state::write_begin (elf_out *to, cpp_reader *reader, table.find_dependencies (this); if (!table.finalize_dependencies ()) - { - to->set_error (); - return; - } + return false; #if CHECKING_P /* We're done verifying at-most once reading, reset to verify @@ -18595,6 +18592,8 @@ module_state::write_begin (elf_out *to, cpp_reader *reader, // so-controlled. if (false) write_env (to); + + return true; } // Finish module writing after we've emitted all dynamic initializers. @@ -20847,22 +20846,27 @@ finish_module_processing (cpp_reader *reader) cookie = new module_processing_cookie (cmi_name, tmp_name, fd, e); + bool report_error = false; if (errorcount /* Don't write the module if it contains an erroneous template. */ || (erroneous_templates && !erroneous_templates->is_empty ())) - warning_at (state->loc, 0, "not writing module %qs due to errors", - state->get_flatname ()); + report_error = true; else if (cookie->out.begin ()) { - cookie->began = true; - auto loc = input_location; /* So crashes finger-point the module decl. */ - input_location = state->loc; - state->write_begin (&cookie->out, reader, cookie->config, cookie->crc); - input_location = loc; + iloc_sentinel ils = state->loc; + if (state->write_begin (&cookie->out, reader, cookie->config, + cookie->crc)) + cookie->began = true; + else + report_error = true; } + if (report_error) + warning_at (state->loc, 0, "not writing module %qs due to errors", + state->get_flatname ()); + dump.pop (n); timevar_stop (TV_MODULE_EXPORT); diff --git a/gcc/testsuite/g++.dg/modules/block-decl-2.C b/gcc/testsuite/g++.dg/modules/block-decl-2.C index 974e26f9b7a..90f18b30945 100644 --- a/gcc/testsuite/g++.dg/modules/block-decl-2.C +++ b/gcc/testsuite/g++.dg/modules/block-decl-2.C @@ -18,4 +18,4 @@ export extern "C++" auto foo() { return X{}; } -// { dg-prune-output "failed to write compiled module" } +// { dg-prune-output "not writing module" } diff --git a/gcc/testsuite/g++.dg/modules/internal-1.C b/gcc/testsuite/g++.dg/modules/internal-1.C index 45d3bf06f28..399dd68b92e 100644 --- a/gcc/testsuite/g++.dg/modules/internal-1.C +++ b/gcc/testsuite/g++.dg/modules/internal-1.C @@ -1,6 +1,6 @@ // { dg-additional-options "-fmodules-ts" } -export module frob; // { dg-error "failed to write" } +export module frob; // { dg-warning "not writing module" } // { dg-module-cmi !frob } namespace {
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? Or should we even just remove the warning entirely? I'm not sure it really adds all that much, since it's usual AFAICT for errors to prevent the intended outputs from being generated. -- >8 -- It was pointed out to me that the current error referencing an internal linkage entity reads almost like an ICE message, with the message finishing with the unhelpful: m.cpp:1:8: error: failed to write compiled module: Bad file data 1 | export module M; | ^~~~~~ It would probably be clearer to just emit the same warning that we do in other cases where we don't write modules due to errors. gcc/cp/ChangeLog: * module.cc (module_state::write_begin): Return a boolean to indicate errors rather than just doing to->set_error(). (finish_module_processing): Check for failed write_begin and disable module writing in that case. gcc/testsuite/ChangeLog: * g++.dg/modules/block-decl-2.C: Adjust error message. * g++.dg/modules/internal-1.C: Likewise. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/module.cc | 30 ++++++++++++--------- gcc/testsuite/g++.dg/modules/block-decl-2.C | 2 +- gcc/testsuite/g++.dg/modules/internal-1.C | 2 +- 3 files changed, 19 insertions(+), 15 deletions(-)