| Message ID | 66f1fe09.050a0220.363eeb.069b@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: Implement P1815 "Translation-unit-local entities" | expand |
On Tue, Sep 24, 2024 at 09:47:17AM +1000, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > -- >8 -- > > [module.import] p6 says "A header unit shall not contain a definition of > a non-inline function or variable whose name has external linkage." > > This patch implements this requirement, and cleans up some issues in the > testsuite where this was already violated. To handle deduction guides > we mark them as inline, since although we give them a definition for > implementation by the standard they have no definition, and so should > not error in this case. > > One remaining question is the behaviour of code like: > > struct S { static const int x = 123; }; > > 'S::x' is not 'inline' here, but this is legal code as long as there is > exactly one definition elsewhere if 'x' is ever odr-used, as specified > by [class.static.data] p4. However, since it's not 'inline' then the > exemption for [module.import] does not apply, and so it appears uses of > this in header units should error. > > Unfortunately the standard library headers do this, and there doesn't > appear to be an easy C++98-compatible way to adjust this. Additionally > I'm not entirely certain that this wasn't an oversight; as such this > patch reduces this specific case to merely a pedwarn. > While testing another change I found that this causes test regressions now since r15-3859-g63a598deb0c9fc; the pedwarn for usages in the standard library headers gets picked up as an error in tests that build the standard library headers as header units (e.g. xtreme-header*, binding*, etc.). What would be the best approach here in your opinion: - Silence the pedwarn in the libstdc++ headers with a #pragma? - Change modules.exp to treat these as system headers again? - Make a dedicated flag for this specific pedwarn and disable it? - Assume that this is meant to be legal and silence the warning entirely? Nathaniel > PR c++/116401 > > gcc/cp/ChangeLog: > > * decl.cc (grokfndecl): Mark deduction guides as 'inline'. > * module.cc (check_module_decl_linkage): Implement checks for > non-inline external linkage definitions in headers. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/macro-4_c.H: Add missing 'inline'. > * g++.dg/modules/pr106761.h: Likewise. > * g++.dg/modules/pr98843_b.H: Likewise. > * g++.dg/modules/pr99468.H: Likewise. > * g++.dg/modules/pragma-1_a.H: Likewise. > * g++.dg/modules/tpl-ary-1.h: Likewise. > * g++.dg/modules/hdr-2.H: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/decl.cc | 1 + > gcc/cp/module.cc | 33 +++++ > gcc/testsuite/g++.dg/modules/hdr-2.H | 164 ++++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/macro-4_c.H | 2 +- > gcc/testsuite/g++.dg/modules/pr106761.h | 2 +- > gcc/testsuite/g++.dg/modules/pr98843_b.H | 2 +- > gcc/testsuite/g++.dg/modules/pr99468.H | 2 +- > gcc/testsuite/g++.dg/modules/pragma-1_a.H | 2 +- > gcc/testsuite/g++.dg/modules/tpl-ary-1.h | 2 +- > 9 files changed, 204 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/hdr-2.H > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 2190ede745b..0203bbb682b 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -10818,6 +10818,7 @@ grokfndecl (tree ctype, > have one: the restriction that you can't repeat a deduction guide > makes them more like a definition anyway. */ > DECL_INITIAL (decl) = void_node; > + DECL_DECLARED_INLINE_P (decl) = true; > break; > default: > break; > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index d7e6fe2c54f..143eb676ce9 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -19937,6 +19937,39 @@ check_module_decl_linkage (tree decl) > if (!module_has_cmi_p ()) > return; > > + /* A header unit shall not contain a definition of a non-inline function > + or variable (not template) whose name has external linkage. */ > + if (header_module_p () > + && !processing_template_decl > + && ((TREE_CODE (decl) == FUNCTION_DECL > + && !DECL_DECLARED_INLINE_P (decl) > + && DECL_INITIAL (decl)) > + || (TREE_CODE (decl) == VAR_DECL > + && !DECL_INLINE_VAR_P (decl) > + && DECL_INITIALIZED_P (decl))) > + && !(DECL_LANG_SPECIFIC (decl) > + && DECL_TEMPLATE_INSTANTIATION (decl)) > + && decl_linkage (decl) == lk_external) > + { > + /* Strictly speaking, > + > + struct S { static const int x = 123; }; > + > + is not valid in a header unit as currently specified. But this is > + done within the standard library, and there doesn't seem to be a > + C++98-compatible alternative, so we support this with a pedwarn. */ > + if (VAR_P (decl) > + && DECL_CLASS_SCOPE_P (decl) > + && DECL_INITIALIZED_IN_CLASS_P (decl)) > + pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic, > + "external linkage definition of %qD in header module must " > + "be declared %<inline%>", decl); > + else > + error_at (DECL_SOURCE_LOCATION (decl), > + "external linkage definition of %qD in header module must " > + "be declared %<inline%>", decl); > + } > + > /* An internal-linkage declaration cannot be generally be exported. > But it's OK to export any declaration from a header unit, including > internal linkage declarations. */ > diff --git a/gcc/testsuite/g++.dg/modules/hdr-2.H b/gcc/testsuite/g++.dg/modules/hdr-2.H > new file mode 100644 > index 00000000000..bb3d7d70123 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/hdr-2.H > @@ -0,0 +1,164 @@ > +// { dg-additional-options "-fmodule-header -Wno-error=pedantic" } > +// { dg-module-cmi !{} } > +// external linkage variables or functions in header units must > +// not have non-inline definitions > + > +int x_err; // { dg-error "inline" } > +int y_err = 123; // { dg-error "inline" } > +void f_err() {} // { dg-error "inline" } > + > +struct Err { > + Err(); > + void m(); > + static void s(); > + static int x; > + static int y; > +}; > +Err::Err() = default; // { dg-error "inline" } > +void Err::m() {} // { dg-error "inline" } > +void Err::s() {} // { dg-error "inline" } > +int Err::x; // { dg-error "inline" } > +int Err::y = 123; // { dg-error "inline" } > + > +// Strictly speaking erroneous, but we support anyway with pedwarn: > +struct Ped { > + enum E { V }; > + static const int x = 123; // { dg-warning "inline" } > + static const E e = V; // { dg-warning "inline" } > +}; > + > +// No definition, OK > +extern int y_decl; > +void f_decl(); > + > +template <typename> struct DeductionGuide {}; > +DeductionGuide() -> DeductionGuide<int>; > + > +// Inline decls are OK > +inline int x_inl; > +inline int y_inl = 123; > +inline void f_inl() {} > +constexpr void g_inl() {} > +void h_inl() = delete; > + > +struct Inl { > + void m() {} > + static void s() {} > + static inline int x; > + static inline int y = 123; > +}; > + > +// Internal linkage decls are OK > +static int x_internal; > +static int y_internal = 123; > +static void f_internal() {} > + > +namespace { > + struct Internal { > + void m(); > + static void s(); > + static int x; > + static int y; > + }; > + void Internal::m() {} > + void Internal::s() {} > + int Internal::x; > + int Internal::y = 123; > +} > + > +// Function-scope entities are OK > +inline void f_static() { > + static int x_static; > + static int y_static = 123; > + thread_local int x_thread_local; > + thread_local int y_thread_local = 123; > + > + x_static = y_static; > + x_thread_local = y_thread_local; > +} > + > +// Templates (not functions or variables) are OK > +template <typename> int x_tpl; > +template <typename> int y_tpl = 123; > +template <typename> void f_tpl() {} > + > +struct Template_Body { > + template <typename> void m(); > + template <typename> static void s(); > + template <typename> static int x; > + template <typename> static int y; > +}; > +template <typename> void Template_Body::m() {} > +template <typename> void Template_Body::s() {} > +template <typename> int Template_Body::x; > +template <typename> int Template_Body::y = 123; > + > +template <typename> struct Template_Type { > + void m(); > + static void s(); > + static int x; > + static int y; > +}; > +template <typename T> void Template_Type<T>::m() {} > +template <typename T> void Template_Type<T>::s() {} > +template <typename T> int Template_Type<T>::x; > +template <typename T> int Template_Type<T>::y = 123; > + > +// Implicit instantiations are OK > +inline void instantiate_tmpls() { > + x_tpl<int> = y_tpl<int>; > + f_tpl<int>(); > + > + Template_Body{}.m<int>(); > + Template_Body::s<int>(); > + Template_Body::x<int> = Template_Body::y<int>; > + > + using TT = Template_Type<int>; > + TT{}.m(); > + TT::s(); > + TT::x = TT::y; > +} > + > +// Explicit instantiations are also OK (extern or otherwise) > +template int x_tpl<char>; > +template int y_tpl<char>; > +template void f_tpl<char>(); > + > +template void Template_Body::m<char>(); > +template void Template_Body::s<char>(); > +template int Template_Body::x<char>; > +template int Template_Body::y<char>; > + > +template void Template_Type<char>::m(); > +template void Template_Type<char>::s(); > +template int Template_Type<char>::x; > +template int Template_Type<char>::y; > + > +extern template int x_tpl<double>; > +extern template int y_tpl<double>; > +extern template void f_tpl<double>(); > + > +extern template void Template_Body::m<double>(); > +extern template void Template_Body::s<double>(); > +extern template int Template_Body::x<double>; > +extern template int Template_Body::y<double>; > + > +extern template void Template_Type<double>::m(); > +extern template void Template_Type<double>::s(); > +extern template int Template_Type<double>::x; > +extern template int Template_Type<double>::y; > + > +// But explicit specialisations are not (though note [temp.expl.spec] p13) > +template <> int x_tpl<short>; // { dg-error "inline" } > +template <> int y_tpl<short> = 123; // { dg-error "inline" } > +template <> void f_tpl<short>() {} // { dg-error "inline" } > + > +template <> void Template_Body::m<short>() {} // { dg-error "inline" } > +template <> void Template_Body::s<short>() {} // { dg-error "inline" } > +template <> int Template_Body::x<short>; // { dg-bogus "inline" "not a definition" } > +template <> int Template_Body::y<short> = 123; // { dg-error "inline" } > + > +template <> void Template_Type<short>::m() {} // { dg-error "inline" } > +template <> void Template_Type<short>::s() {} // { dg-error "inline" } > +template <> int Template_Type<short>::x; // { dg-bogus "inline" "not a definition" } > +template <> int Template_Type<short>::y = 123; // { dg-error "inline" } > diff --git a/gcc/testsuite/g++.dg/modules/macro-4_c.H b/gcc/testsuite/g++.dg/modules/macro-4_c.H > index ec2bed91ccd..5692e8e41d2 100644 > --- a/gcc/testsuite/g++.dg/modules/macro-4_c.H > +++ b/gcc/testsuite/g++.dg/modules/macro-4_c.H > @@ -6,7 +6,7 @@ > > #undef FIVE // no effect > import "macro-4_a.H"; > -int a; > +inline int a; > #undef THREE > #undef FOUR > #define FOUR 4c > diff --git a/gcc/testsuite/g++.dg/modules/pr106761.h b/gcc/testsuite/g++.dg/modules/pr106761.h > index 9f22a22a45d..5c13fc0f118 100644 > --- a/gcc/testsuite/g++.dg/modules/pr106761.h > +++ b/gcc/testsuite/g++.dg/modules/pr106761.h > @@ -19,4 +19,4 @@ struct tuple { > = typename _TupleConstraints<Ts...>::template __constructible<Us...>; > }; > > -tuple<int, int> t; > +inline tuple<int, int> t; > diff --git a/gcc/testsuite/g++.dg/modules/pr98843_b.H b/gcc/testsuite/g++.dg/modules/pr98843_b.H > index d6734bd382d..af504a840c7 100644 > --- a/gcc/testsuite/g++.dg/modules/pr98843_b.H > +++ b/gcc/testsuite/g++.dg/modules/pr98843_b.H > @@ -6,7 +6,7 @@ template<int I> int Fn () > return I; > } > > -template<> int Fn<1> () > +template<> inline int Fn<1> () > { > return 0; > } > diff --git a/gcc/testsuite/g++.dg/modules/pr99468.H b/gcc/testsuite/g++.dg/modules/pr99468.H > index d7da3a83e1c..df63c613b2c 100644 > --- a/gcc/testsuite/g++.dg/modules/pr99468.H > +++ b/gcc/testsuite/g++.dg/modules/pr99468.H > @@ -3,4 +3,4 @@ > > module M; // { dg-error "module-declaration not permitted" } > > -int i; > +inline int i; > diff --git a/gcc/testsuite/g++.dg/modules/pragma-1_a.H b/gcc/testsuite/g++.dg/modules/pragma-1_a.H > index 6eb0a5900a7..a69be2fd05e 100644 > --- a/gcc/testsuite/g++.dg/modules/pragma-1_a.H > +++ b/gcc/testsuite/g++.dg/modules/pragma-1_a.H > @@ -1,4 +1,4 @@ > // { dg-additional-options -fmodule-header } > > // { dg-module-cmi {} } > -int i; > +inline int i; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h > index 2f745afba36..2f9a8106412 100644 > --- a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h > +++ b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h > @@ -1,5 +1,5 @@ > > -int ary[4]; > +inline int ary[4]; > extern int unb[]; > typedef int z[0]; > > -- > 2.46.0 >
On 9/26/24 1:29 AM, Nathaniel Shead wrote: > On Tue, Sep 24, 2024 at 09:47:17AM +1000, Nathaniel Shead wrote: >> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? >> >> -- >8 -- >> >> [module.import] p6 says "A header unit shall not contain a definition of >> a non-inline function or variable whose name has external linkage." >> >> This patch implements this requirement, and cleans up some issues in the >> testsuite where this was already violated. To handle deduction guides >> we mark them as inline, since although we give them a definition for >> implementation by the standard they have no definition, and so should >> not error in this case. >> >> One remaining question is the behaviour of code like: >> >> struct S { static const int x = 123; }; >> >> 'S::x' is not 'inline' here, but this is legal code as long as there is >> exactly one definition elsewhere if 'x' is ever odr-used, as specified >> by [class.static.data] p4. However, since it's not 'inline' then the >> exemption for [module.import] does not apply, and so it appears uses of >> this in header units should error. But that isn't a definition, only a declaration. So it's well-formed in a header unit. https://eel.is/c++draft/class.static#data-3 Jason
On Thu, Sep 26, 2024 at 05:27:23PM -0400, Jason Merrill wrote: > On 9/26/24 1:29 AM, Nathaniel Shead wrote: > > On Tue, Sep 24, 2024 at 09:47:17AM +1000, Nathaniel Shead wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > > > -- >8 -- > > > > > > [module.import] p6 says "A header unit shall not contain a definition of > > > a non-inline function or variable whose name has external linkage." > > > > > > This patch implements this requirement, and cleans up some issues in the > > > testsuite where this was already violated. To handle deduction guides > > > we mark them as inline, since although we give them a definition for > > > implementation by the standard they have no definition, and so should > > > not error in this case. > > > > > > One remaining question is the behaviour of code like: > > > > > > struct S { static const int x = 123; }; > > > > > > 'S::x' is not 'inline' here, but this is legal code as long as there is > > > exactly one definition elsewhere if 'x' is ever odr-used, as specified > > > by [class.static.data] p4. However, since it's not 'inline' then the > > > exemption for [module.import] does not apply, and so it appears uses of > > > this in header units should error. > > But that isn't a definition, only a declaration. So it's well-formed in a > header unit. > > https://eel.is/c++draft/class.static#data-3 > > Jason > Ah right, thanks, completely missed that. I'll update this in the next version of this patch series I'll be sending out shortly. Nathaniel
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 2190ede745b..0203bbb682b 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -10818,6 +10818,7 @@ grokfndecl (tree ctype, have one: the restriction that you can't repeat a deduction guide makes them more like a definition anyway. */ DECL_INITIAL (decl) = void_node; + DECL_DECLARED_INLINE_P (decl) = true; break; default: break; diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index d7e6fe2c54f..143eb676ce9 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -19937,6 +19937,39 @@ check_module_decl_linkage (tree decl) if (!module_has_cmi_p ()) return; + /* A header unit shall not contain a definition of a non-inline function + or variable (not template) whose name has external linkage. */ + if (header_module_p () + && !processing_template_decl + && ((TREE_CODE (decl) == FUNCTION_DECL + && !DECL_DECLARED_INLINE_P (decl) + && DECL_INITIAL (decl)) + || (TREE_CODE (decl) == VAR_DECL + && !DECL_INLINE_VAR_P (decl) + && DECL_INITIALIZED_P (decl))) + && !(DECL_LANG_SPECIFIC (decl) + && DECL_TEMPLATE_INSTANTIATION (decl)) + && decl_linkage (decl) == lk_external) + { + /* Strictly speaking, + + struct S { static const int x = 123; }; + + is not valid in a header unit as currently specified. But this is + done within the standard library, and there doesn't seem to be a + C++98-compatible alternative, so we support this with a pedwarn. */ + if (VAR_P (decl) + && DECL_CLASS_SCOPE_P (decl) + && DECL_INITIALIZED_IN_CLASS_P (decl)) + pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic, + "external linkage definition of %qD in header module must " + "be declared %<inline%>", decl); + else + error_at (DECL_SOURCE_LOCATION (decl), + "external linkage definition of %qD in header module must " + "be declared %<inline%>", decl); + } + /* An internal-linkage declaration cannot be generally be exported. But it's OK to export any declaration from a header unit, including internal linkage declarations. */ diff --git a/gcc/testsuite/g++.dg/modules/hdr-2.H b/gcc/testsuite/g++.dg/modules/hdr-2.H new file mode 100644 index 00000000000..bb3d7d70123 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/hdr-2.H @@ -0,0 +1,164 @@ +// { dg-additional-options "-fmodule-header -Wno-error=pedantic" } +// { dg-module-cmi !{} } +// external linkage variables or functions in header units must +// not have non-inline definitions + +int x_err; // { dg-error "inline" } +int y_err = 123; // { dg-error "inline" } +void f_err() {} // { dg-error "inline" } + +struct Err { + Err(); + void m(); + static void s(); + static int x; + static int y; +}; +Err::Err() = default; // { dg-error "inline" } +void Err::m() {} // { dg-error "inline" } +void Err::s() {} // { dg-error "inline" } +int Err::x; // { dg-error "inline" } +int Err::y = 123; // { dg-error "inline" } + +// Strictly speaking erroneous, but we support anyway with pedwarn: +struct Ped { + enum E { V }; + static const int x = 123; // { dg-warning "inline" } + static const E e = V; // { dg-warning "inline" } +}; + +// No definition, OK +extern int y_decl; +void f_decl(); + +template <typename> struct DeductionGuide {}; +DeductionGuide() -> DeductionGuide<int>; + +// Inline decls are OK +inline int x_inl; +inline int y_inl = 123; +inline void f_inl() {} +constexpr void g_inl() {} +void h_inl() = delete; + +struct Inl { + void m() {} + static void s() {} + static inline int x; + static inline int y = 123; +}; + +// Internal linkage decls are OK +static int x_internal; +static int y_internal = 123; +static void f_internal() {} + +namespace { + struct Internal { + void m(); + static void s(); + static int x; + static int y; + }; + void Internal::m() {} + void Internal::s() {} + int Internal::x; + int Internal::y = 123; +} + +// Function-scope entities are OK +inline void f_static() { + static int x_static; + static int y_static = 123; + thread_local int x_thread_local; + thread_local int y_thread_local = 123; + + x_static = y_static; + x_thread_local = y_thread_local; +} + +// Templates (not functions or variables) are OK +template <typename> int x_tpl; +template <typename> int y_tpl = 123; +template <typename> void f_tpl() {} + +struct Template_Body { + template <typename> void m(); + template <typename> static void s(); + template <typename> static int x; + template <typename> static int y; +}; +template <typename> void Template_Body::m() {} +template <typename> void Template_Body::s() {} +template <typename> int Template_Body::x; +template <typename> int Template_Body::y = 123; + +template <typename> struct Template_Type { + void m(); + static void s(); + static int x; + static int y; +}; +template <typename T> void Template_Type<T>::m() {} +template <typename T> void Template_Type<T>::s() {} +template <typename T> int Template_Type<T>::x; +template <typename T> int Template_Type<T>::y = 123; + +// Implicit instantiations are OK +inline void instantiate_tmpls() { + x_tpl<int> = y_tpl<int>; + f_tpl<int>(); + + Template_Body{}.m<int>(); + Template_Body::s<int>(); + Template_Body::x<int> = Template_Body::y<int>; + + using TT = Template_Type<int>; + TT{}.m(); + TT::s(); + TT::x = TT::y; +} + +// Explicit instantiations are also OK (extern or otherwise) +template int x_tpl<char>; +template int y_tpl<char>; +template void f_tpl<char>(); + +template void Template_Body::m<char>(); +template void Template_Body::s<char>(); +template int Template_Body::x<char>; +template int Template_Body::y<char>; + +template void Template_Type<char>::m(); +template void Template_Type<char>::s(); +template int Template_Type<char>::x; +template int Template_Type<char>::y; + +extern template int x_tpl<double>; +extern template int y_tpl<double>; +extern template void f_tpl<double>(); + +extern template void Template_Body::m<double>(); +extern template void Template_Body::s<double>(); +extern template int Template_Body::x<double>; +extern template int Template_Body::y<double>; + +extern template void Template_Type<double>::m(); +extern template void Template_Type<double>::s(); +extern template int Template_Type<double>::x; +extern template int Template_Type<double>::y; + +// But explicit specialisations are not (though note [temp.expl.spec] p13) +template <> int x_tpl<short>; // { dg-error "inline" } +template <> int y_tpl<short> = 123; // { dg-error "inline" } +template <> void f_tpl<short>() {} // { dg-error "inline" } + +template <> void Template_Body::m<short>() {} // { dg-error "inline" } +template <> void Template_Body::s<short>() {} // { dg-error "inline" } +template <> int Template_Body::x<short>; // { dg-bogus "inline" "not a definition" } +template <> int Template_Body::y<short> = 123; // { dg-error "inline" } + +template <> void Template_Type<short>::m() {} // { dg-error "inline" } +template <> void Template_Type<short>::s() {} // { dg-error "inline" } +template <> int Template_Type<short>::x; // { dg-bogus "inline" "not a definition" } +template <> int Template_Type<short>::y = 123; // { dg-error "inline" } diff --git a/gcc/testsuite/g++.dg/modules/macro-4_c.H b/gcc/testsuite/g++.dg/modules/macro-4_c.H index ec2bed91ccd..5692e8e41d2 100644 --- a/gcc/testsuite/g++.dg/modules/macro-4_c.H +++ b/gcc/testsuite/g++.dg/modules/macro-4_c.H @@ -6,7 +6,7 @@ #undef FIVE // no effect import "macro-4_a.H"; -int a; +inline int a; #undef THREE #undef FOUR #define FOUR 4c diff --git a/gcc/testsuite/g++.dg/modules/pr106761.h b/gcc/testsuite/g++.dg/modules/pr106761.h index 9f22a22a45d..5c13fc0f118 100644 --- a/gcc/testsuite/g++.dg/modules/pr106761.h +++ b/gcc/testsuite/g++.dg/modules/pr106761.h @@ -19,4 +19,4 @@ struct tuple { = typename _TupleConstraints<Ts...>::template __constructible<Us...>; }; -tuple<int, int> t; +inline tuple<int, int> t; diff --git a/gcc/testsuite/g++.dg/modules/pr98843_b.H b/gcc/testsuite/g++.dg/modules/pr98843_b.H index d6734bd382d..af504a840c7 100644 --- a/gcc/testsuite/g++.dg/modules/pr98843_b.H +++ b/gcc/testsuite/g++.dg/modules/pr98843_b.H @@ -6,7 +6,7 @@ template<int I> int Fn () return I; } -template<> int Fn<1> () +template<> inline int Fn<1> () { return 0; } diff --git a/gcc/testsuite/g++.dg/modules/pr99468.H b/gcc/testsuite/g++.dg/modules/pr99468.H index d7da3a83e1c..df63c613b2c 100644 --- a/gcc/testsuite/g++.dg/modules/pr99468.H +++ b/gcc/testsuite/g++.dg/modules/pr99468.H @@ -3,4 +3,4 @@ module M; // { dg-error "module-declaration not permitted" } -int i; +inline int i; diff --git a/gcc/testsuite/g++.dg/modules/pragma-1_a.H b/gcc/testsuite/g++.dg/modules/pragma-1_a.H index 6eb0a5900a7..a69be2fd05e 100644 --- a/gcc/testsuite/g++.dg/modules/pragma-1_a.H +++ b/gcc/testsuite/g++.dg/modules/pragma-1_a.H @@ -1,4 +1,4 @@ // { dg-additional-options -fmodule-header } // { dg-module-cmi {} } -int i; +inline int i; diff --git a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h index 2f745afba36..2f9a8106412 100644 --- a/gcc/testsuite/g++.dg/modules/tpl-ary-1.h +++ b/gcc/testsuite/g++.dg/modules/tpl-ary-1.h @@ -1,5 +1,5 @@ -int ary[4]; +inline int ary[4]; extern int unb[]; typedef int z[0];
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- [module.import] p6 says "A header unit shall not contain a definition of a non-inline function or variable whose name has external linkage." This patch implements this requirement, and cleans up some issues in the testsuite where this was already violated. To handle deduction guides we mark them as inline, since although we give them a definition for implementation by the standard they have no definition, and so should not error in this case. One remaining question is the behaviour of code like: struct S { static const int x = 123; }; 'S::x' is not 'inline' here, but this is legal code as long as there is exactly one definition elsewhere if 'x' is ever odr-used, as specified by [class.static.data] p4. However, since it's not 'inline' then the exemption for [module.import] does not apply, and so it appears uses of this in header units should error. Unfortunately the standard library headers do this, and there doesn't appear to be an easy C++98-compatible way to adjust this. Additionally I'm not entirely certain that this wasn't an oversight; as such this patch reduces this specific case to merely a pedwarn. PR c++/116401 gcc/cp/ChangeLog: * decl.cc (grokfndecl): Mark deduction guides as 'inline'. * module.cc (check_module_decl_linkage): Implement checks for non-inline external linkage definitions in headers. gcc/testsuite/ChangeLog: * g++.dg/modules/macro-4_c.H: Add missing 'inline'. * g++.dg/modules/pr106761.h: Likewise. * g++.dg/modules/pr98843_b.H: Likewise. * g++.dg/modules/pr99468.H: Likewise. * g++.dg/modules/pragma-1_a.H: Likewise. * g++.dg/modules/tpl-ary-1.h: Likewise. * g++.dg/modules/hdr-2.H: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/decl.cc | 1 + gcc/cp/module.cc | 33 +++++ gcc/testsuite/g++.dg/modules/hdr-2.H | 164 ++++++++++++++++++++++ gcc/testsuite/g++.dg/modules/macro-4_c.H | 2 +- gcc/testsuite/g++.dg/modules/pr106761.h | 2 +- gcc/testsuite/g++.dg/modules/pr98843_b.H | 2 +- gcc/testsuite/g++.dg/modules/pr99468.H | 2 +- gcc/testsuite/g++.dg/modules/pragma-1_a.H | 2 +- gcc/testsuite/g++.dg/modules/tpl-ary-1.h | 2 +- 9 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/hdr-2.H