| Message ID | 664181e2.650a0220.dbfd8.e3d5@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: Remember that header units have CMIs | expand |
On 5/12/24 22:58, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? OK. > -- >8 -- > > This appears to be an oversight in the definition of module_has_cmi_p; > this comes up transitively in other functions used for e.g. determining > whether a name could potentially be accessed in a different translation > unit. > > This change will allow us to use the function directly in more places > that need to additional work only if generating a module CMI in the > future. > > gcc/cp/ChangeLog: > > * cp-tree.h (module_has_cmi_p): Also true for header units. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/linkage-3_a.H: New test. > * g++.dg/modules/linkage-3_b.C: New test. > * g++.dg/modules/linkage-3_c.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/cp-tree.h | 2 +- > gcc/testsuite/g++.dg/modules/linkage-3_a.H | 19 +++++++++++++++++++ > gcc/testsuite/g++.dg/modules/linkage-3_b.C | 9 +++++++++ > gcc/testsuite/g++.dg/modules/linkage-3_c.C | 10 ++++++++++ > 4 files changed, 39 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_c.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index db098c32f2d..609904918ba 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7379,7 +7379,7 @@ inline bool module_interface_p () > inline bool module_partition_p () > { return module_kind & MK_PARTITION; } > inline bool module_has_cmi_p () > -{ return module_kind & (MK_INTERFACE | MK_PARTITION); } > +{ return module_kind & (MK_INTERFACE | MK_PARTITION | MK_HEADER); } > > inline bool module_purview_p () > { return module_kind & MK_PURVIEW; } > diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_a.H b/gcc/testsuite/g++.dg/modules/linkage-3_a.H > new file mode 100644 > index 00000000000..1e0ebd927e2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-3_a.H > @@ -0,0 +1,19 @@ > +// { dg-additional-options "-fmodule-header -Wno-error=c++20-extensions" } > +// { dg-module-cmi {} } > + > +// Like linkage-1, but for header units. > + > +// External linkage definitions must be declared as 'inline' to satisfy > +// [module.import] p6, so we don't need to care about voldemort types in > +// function definitions. > + > +// Strictly speaking this is not required to be supported: > +// [module.import] p5 says that when two different TUs import header-names > +// identifying the same header or source file, it is unspecified whether > +// they import the same header unit, and thus 's' could be a different entity > +// in each TU. But with out current implementation this seems to reasonable to > +// allow (and it does currently work). > + > +struct { int y; } s; > +decltype(s) f(); // { dg-warning "used but not defined" "" { target c++17_down } } > +inline auto x = f(); > diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_b.C b/gcc/testsuite/g++.dg/modules/linkage-3_b.C > new file mode 100644 > index 00000000000..935ef6150ec > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-3_b.C > @@ -0,0 +1,9 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +struct {} unrelated; > + > +import "linkage-3_a.H"; > + > +decltype(s) f() { > + return { 123 }; > +} > diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_c.C b/gcc/testsuite/g++.dg/modules/linkage-3_c.C > new file mode 100644 > index 00000000000..be527ff552d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-3_c.C > @@ -0,0 +1,10 @@ > +// { dg-module-do run } > +// { dg-additional-options "-fmodules-ts" } > + > +import "linkage-3_a.H"; > + > +int main() { > + auto a = x.y; > + if (a != 123) > + __builtin_abort(); > +}
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index db098c32f2d..609904918ba 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7379,7 +7379,7 @@ inline bool module_interface_p () inline bool module_partition_p () { return module_kind & MK_PARTITION; } inline bool module_has_cmi_p () -{ return module_kind & (MK_INTERFACE | MK_PARTITION); } +{ return module_kind & (MK_INTERFACE | MK_PARTITION | MK_HEADER); } inline bool module_purview_p () { return module_kind & MK_PURVIEW; } diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_a.H b/gcc/testsuite/g++.dg/modules/linkage-3_a.H new file mode 100644 index 00000000000..1e0ebd927e2 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-3_a.H @@ -0,0 +1,19 @@ +// { dg-additional-options "-fmodule-header -Wno-error=c++20-extensions" } +// { dg-module-cmi {} } + +// Like linkage-1, but for header units. + +// External linkage definitions must be declared as 'inline' to satisfy +// [module.import] p6, so we don't need to care about voldemort types in +// function definitions. + +// Strictly speaking this is not required to be supported: +// [module.import] p5 says that when two different TUs import header-names +// identifying the same header or source file, it is unspecified whether +// they import the same header unit, and thus 's' could be a different entity +// in each TU. But with out current implementation this seems to reasonable to +// allow (and it does currently work). + +struct { int y; } s; +decltype(s) f(); // { dg-warning "used but not defined" "" { target c++17_down } } +inline auto x = f(); diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_b.C b/gcc/testsuite/g++.dg/modules/linkage-3_b.C new file mode 100644 index 00000000000..935ef6150ec --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-3_b.C @@ -0,0 +1,9 @@ +// { dg-additional-options "-fmodules-ts" } + +struct {} unrelated; + +import "linkage-3_a.H"; + +decltype(s) f() { + return { 123 }; +} diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_c.C b/gcc/testsuite/g++.dg/modules/linkage-3_c.C new file mode 100644 index 00000000000..be527ff552d --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-3_c.C @@ -0,0 +1,10 @@ +// { dg-module-do run } +// { dg-additional-options "-fmodules-ts" } + +import "linkage-3_a.H"; + +int main() { + auto a = x.y; + if (a != 123) + __builtin_abort(); +}
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This appears to be an oversight in the definition of module_has_cmi_p; this comes up transitively in other functions used for e.g. determining whether a name could potentially be accessed in a different translation unit. This change will allow us to use the function directly in more places that need to additional work only if generating a module CMI in the future. gcc/cp/ChangeLog: * cp-tree.h (module_has_cmi_p): Also true for header units. gcc/testsuite/ChangeLog: * g++.dg/modules/linkage-3_a.H: New test. * g++.dg/modules/linkage-3_b.C: New test. * g++.dg/modules/linkage-3_c.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/cp-tree.h | 2 +- gcc/testsuite/g++.dg/modules/linkage-3_a.H | 19 +++++++++++++++++++ gcc/testsuite/g++.dg/modules/linkage-3_b.C | 9 +++++++++ gcc/testsuite/g++.dg/modules/linkage-3_c.C | 10 ++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_a.H create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_b.C create mode 100644 gcc/testsuite/g++.dg/modules/linkage-3_c.C