| Message ID | 66c72645.050a0220.285d91.334f@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] c++/modules: Clean up include translation [PR110980] | expand |
On 8/22/24 7:51 AM, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? OK. > -- >8 -- > > After importing a header unit we learn about and setup any header > modules that we transitively depend on. However, this causes > 'set_filename' to fail an assertion if we then come across this header > as an #include and attempt to translate it into a module. We still need > to do this translation so that libcpp learns that this is a header unit, > but we shouldn't error just because we've already seen it as an import. > > Instead this patch merely checks and errors to handle the case of a > broken mapper implementation which supplies a different CMI path from > the one we already got. > > As a drive-by fix, also make failing to find the CMI for a module be a > fatal error: any further errors in the TU are unlikely to be helpful. > > PR c++/99243 > > gcc/cp/ChangeLog: > > * module.cc (module_state::set_filename): Handle repeated calls > to 'set_filename' as long as the CMI path matches. > (maybe_translate_include): Adjust comment. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/map-2.C: Prune additional fatal error message. > * g++.dg/modules/inc-xlate-4_a.H: New test. > * g++.dg/modules/inc-xlate-4_b.H: New test. > * g++.dg/modules/inc-xlate-4_c.H: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/module.cc | 18 +++++++++++++----- > gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H | 5 +++++ > gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H | 5 +++++ > gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H | 6 ++++++ > gcc/testsuite/g++.dg/modules/map-2.C | 3 ++- > 5 files changed, 31 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H > create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 4cd7e1c284b..95c2405fcd4 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -20086,14 +20086,21 @@ canonicalize_header_name (cpp_reader *reader, location_t loc, bool unquoted, > > void module_state::set_filename (const Cody::Packet &packet) > { > - gcc_checking_assert (!filename); > if (packet.GetCode () == Cody::Client::PC_PATHNAME) > - filename = xstrdup (packet.GetString ().c_str ()); > + { > + /* If we've seen this import before we better have the same CMI. */ > + const std::string &path = packet.GetString (); > + if (!filename) > + filename = xstrdup (packet.GetString ().c_str ()); > + else if (filename != path) > + error_at (loc, "mismatching compiled module interface: " > + "had %qs, got %qs", filename, path.c_str ()); > + } > else > { > gcc_checking_assert (packet.GetCode () == Cody::Client::PC_ERROR); > - error_at (loc, "unknown Compiled Module Interface: %s", > - packet.GetString ().c_str ()); > + fatal_error (loc, "unknown compiled module interface: %s", > + packet.GetString ().c_str ()); > } > } > > @@ -20127,7 +20134,8 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc, > translate = packet.GetInteger () ? xlate_kind::text : xlate_kind::unknown; > else if (packet.GetCode () == Cody::Client::PC_PATHNAME) > { > - /* Record the CMI name for when we do the import. */ > + /* Record the CMI name for when we do the import. > + We may already know about this import, but libcpp doesn't yet. */ > module_state *import = get_module (build_string (len, path)); > import->set_filename (packet); > translate = xlate_kind::import; > diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H > new file mode 100644 > index 00000000000..8afb49d01a5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H > @@ -0,0 +1,5 @@ > +// PR c++/99243 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +void foo(); > diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H > new file mode 100644 > index 00000000000..0e67566f571 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H > @@ -0,0 +1,5 @@ > +// PR c++/99243 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +#include "inc-xlate-4_a.H" > diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H > new file mode 100644 > index 00000000000..c2fa647bce8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H > @@ -0,0 +1,6 @@ > +// PR c++/99243 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +#include "inc-xlate-4_b.H" > +#include "inc-xlate-4_a.H" > diff --git a/gcc/testsuite/g++.dg/modules/map-2.C b/gcc/testsuite/g++.dg/modules/map-2.C > index 94d3f7a1a41..3f95aea3670 100644 > --- a/gcc/testsuite/g++.dg/modules/map-2.C > +++ b/gcc/testsuite/g++.dg/modules/map-2.C > @@ -5,5 +5,6 @@ > // { dg-additional-files map-2.map } > > export module foo; > -// { dg-error "Interface: no such module" "" { target *-*-* } .-1 } > +// { dg-error "interface: no such module" "" { target *-*-* } .-1 } > // { dg-error "failed reading mapper" "" { target *-*-* } 0 } > +// { dg-prune-output "compilation terminated" }
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 4cd7e1c284b..95c2405fcd4 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -20086,14 +20086,21 @@ canonicalize_header_name (cpp_reader *reader, location_t loc, bool unquoted, void module_state::set_filename (const Cody::Packet &packet) { - gcc_checking_assert (!filename); if (packet.GetCode () == Cody::Client::PC_PATHNAME) - filename = xstrdup (packet.GetString ().c_str ()); + { + /* If we've seen this import before we better have the same CMI. */ + const std::string &path = packet.GetString (); + if (!filename) + filename = xstrdup (packet.GetString ().c_str ()); + else if (filename != path) + error_at (loc, "mismatching compiled module interface: " + "had %qs, got %qs", filename, path.c_str ()); + } else { gcc_checking_assert (packet.GetCode () == Cody::Client::PC_ERROR); - error_at (loc, "unknown Compiled Module Interface: %s", - packet.GetString ().c_str ()); + fatal_error (loc, "unknown compiled module interface: %s", + packet.GetString ().c_str ()); } } @@ -20127,7 +20134,8 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc, translate = packet.GetInteger () ? xlate_kind::text : xlate_kind::unknown; else if (packet.GetCode () == Cody::Client::PC_PATHNAME) { - /* Record the CMI name for when we do the import. */ + /* Record the CMI name for when we do the import. + We may already know about this import, but libcpp doesn't yet. */ module_state *import = get_module (build_string (len, path)); import->set_filename (packet); translate = xlate_kind::import; diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H new file mode 100644 index 00000000000..8afb49d01a5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H @@ -0,0 +1,5 @@ +// PR c++/99243 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +void foo(); diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H new file mode 100644 index 00000000000..0e67566f571 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H @@ -0,0 +1,5 @@ +// PR c++/99243 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +#include "inc-xlate-4_a.H" diff --git a/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H b/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H new file mode 100644 index 00000000000..c2fa647bce8 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H @@ -0,0 +1,6 @@ +// PR c++/99243 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +#include "inc-xlate-4_b.H" +#include "inc-xlate-4_a.H" diff --git a/gcc/testsuite/g++.dg/modules/map-2.C b/gcc/testsuite/g++.dg/modules/map-2.C index 94d3f7a1a41..3f95aea3670 100644 --- a/gcc/testsuite/g++.dg/modules/map-2.C +++ b/gcc/testsuite/g++.dg/modules/map-2.C @@ -5,5 +5,6 @@ // { dg-additional-files map-2.map } export module foo; -// { dg-error "Interface: no such module" "" { target *-*-* } .-1 } +// { dg-error "interface: no such module" "" { target *-*-* } .-1 } // { dg-error "failed reading mapper" "" { target *-*-* } 0 } +// { dg-prune-output "compilation terminated" }
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- After importing a header unit we learn about and setup any header modules that we transitively depend on. However, this causes 'set_filename' to fail an assertion if we then come across this header as an #include and attempt to translate it into a module. We still need to do this translation so that libcpp learns that this is a header unit, but we shouldn't error just because we've already seen it as an import. Instead this patch merely checks and errors to handle the case of a broken mapper implementation which supplies a different CMI path from the one we already got. As a drive-by fix, also make failing to find the CMI for a module be a fatal error: any further errors in the TU are unlikely to be helpful. PR c++/99243 gcc/cp/ChangeLog: * module.cc (module_state::set_filename): Handle repeated calls to 'set_filename' as long as the CMI path matches. (maybe_translate_include): Adjust comment. gcc/testsuite/ChangeLog: * g++.dg/modules/map-2.C: Prune additional fatal error message. * g++.dg/modules/inc-xlate-4_a.H: New test. * g++.dg/modules/inc-xlate-4_b.H: New test. * g++.dg/modules/inc-xlate-4_c.H: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/module.cc | 18 +++++++++++++----- gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H | 5 +++++ gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H | 5 +++++ gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H | 6 ++++++ gcc/testsuite/g++.dg/modules/map-2.C | 3 ++- 5 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_a.H create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_b.H create mode 100644 gcc/testsuite/g++.dg/modules/inc-xlate-4_c.H