| Message ID | 65f58152.620a0220.6c9e6.eb75@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] c++: Fix handling of no-linkage decls for modules | expand |
On 3/16/24 07:23, Nathaniel Shead wrote: > On Mon, Mar 11, 2024 at 02:13:34PM -0400, Jason Merrill wrote: >> On 3/8/24 18:18, Nathaniel Shead wrote: >>> On Fri, Mar 08, 2024 at 10:19:52AM -0500, Jason Merrill wrote: >>>> On 3/7/24 21:55, Nathaniel Shead wrote: >>>>> On Mon, Nov 27, 2023 at 03:59:39PM +1100, Nathaniel Shead wrote: >>>>>> On Thu, Nov 23, 2023 at 03:03:37PM -0500, Nathan Sidwell wrote: >>>>>>> On 11/20/23 04:47, Nathaniel Shead wrote: >>>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write >>>>>>>> access. >>>>>>>> >>>>>>>> -- >8 -- >>>>>>>> >>>>>>>> Block-scope declarations of functions or extern values are not allowed >>>>>>>> when attached to a named module. Similarly, class member functions are >>>>>>>> not inline if attached to a named module. However, in both these cases >>>>>>>> we currently only check if the declaration is within the module purview; >>>>>>>> it is possible for such a declaration to occur within the module purview >>>>>>>> but not be attached to a named module (e.g. in an 'extern "C++"' block). >>>>>>>> This patch makes the required adjustments. >>>>>>> >>>>>>> >>>>>>> Ah I'd been puzzling over the default inlinedness of member-fns of >>>>>>> block-scope structs. Could you augment the testcase to make sure that's >>>>>>> right too? >>>>>>> >>>>>>> Something like: >>>>>>> >>>>>>> // dg-module-do link >>>>>>> export module Mod; >>>>>>> >>>>>>> export auto Get () { >>>>>>> struct X { void Fn () {} }; >>>>>>> return X(); >>>>>>> } >>>>>>> >>>>>>> >>>>>>> /// >>>>>>> import Mod >>>>>>> void Frob () { Get().Fn(); } >>>>>>> >>>>>> >>>>>> I gave this a try and it indeed doesn't work correctly; 'Fn' needs to be >>>>>> marked 'inline' for this to link (whether or not 'Get' itself is >>>>>> inline). I've tried tracing the code to work out what's going on but >>>>>> I've been struggling to work out how all the different flags (e.g. >>>>>> TREE_PUBLIC, TREE_EXTERNAL, TREE_COMDAT, DECL_NOT_REALLY_EXTERN) >>>>>> interact, which flags we want to be set where, and where the decision of >>>>>> what function definitions to emit is actually made. >>>>>> >>>>>> I did find that doing 'mark_used(decl)' on all member functions in >>>>>> block-scope structs seems to work however, but I wonder if that's maybe >>>>>> too aggressive or if there's something else we should be doing? >>>>> >>>>> I got around to looking at this again, here's an updated version of this >>>>> patch. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? >>>>> >>>>> (I'm not sure if 'start_preparsed_function' is the right place to be >>>>> putting this kind of logic or if it should instead be going in >>>>> 'grokfndecl', e.g. decl.cc:10761 where the rules for making local >>>>> functions have no linkage are initially determined, but I found this >>>>> easier to implement: happy to move around though if preferred.) >>>>> >>>>> -- >8 -- >>>>> >>>>> Block-scope declarations of functions or extern values are not allowed >>>>> when attached to a named module. Similarly, class member functions are >>>>> not inline if attached to a named module. However, in both these cases >>>>> we currently only check if the declaration is within the module purview; >>>>> it is possible for such a declaration to occur within the module purview >>>>> but not be attached to a named module (e.g. in an 'extern "C++"' block). >>>>> This patch makes the required adjustments. >>>>> >>>>> While implementing this we discovered that block-scope local functions >>>>> are not correctly emitted, causing link failures; this patch also >>>>> corrects some assumptions here and ensures that they are emitted when >>>>> needed. >>>>> >>>>> PR c++/112631 >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> * cp-tree.h (named_module_attach_p): New function. >>>>> * decl.cc (start_decl): Check for attachment not purview. >>>>> (grokmethod): Likewise. >>>> >>>> These changes are OK; the others I want to consider more. >>> >>> Thanks, I can commit this as a separate commit if you prefer? >> >> Please. >> > > Thanks, committed as r14-9501-gead3075406ece9. > >>>>> +export auto n_n() { >>>>> + internal(); >>>>> + struct X { void f() { internal(); } }; >>>>> + return X{}; >>>> >>>> Hmm, is this not a prohibited exposure? Seems like X has no linkage because >>>> it's at block scope, and the deduced return type names it. >>>> >>>> I know we try to support this "voldemort" pattern, but is that actually >>>> correct? >>> >>> I had similar doubts, but this is not an especially uncommon pattern in >>> the wild either. I also asked some other people for their thoughts and >>> got told: >>> >>> "no linkage" doesn't mean it's ill-formed to name it in other scopes. >>> It means a declaration in another scope cannot correspond to it >>> >>> And that the wording in [basic.link] p2.4 is imprecise. (Apparently they >>> were going to raise a core issue about this too, I think?) >>> >>> As for whether it's an exposure, looking at [basic.link] p15, the entity >>> 'X' doesn't actually appear to be TU-local: it doesn't have a name with >>> internal linkage (no linkage is different) and is not declared or >>> introduced within the definition of a TU-local entity (n_n is not >>> TU-local). >> >> Hmm, I think you're right. And this rule: >> >>> - /* DR 757: A type without linkage shall not be used as the type of a >>> - variable or function with linkage, unless >>> - o the variable or function has extern "C" linkage (7.5 [dcl.link]), or >>> - o the variable or function is not used (3.2 [basic.def.odr]) or is >>> - defined in the same translation unit. >> >> is no longer part of the standard since C++20; the remnant of this rule is >> the example in >> >> https://eel.is/c++draft/basic#def.odr-11 >> >>> auto f() { >>> struct A {}; >>> return A{}; >>> } >>> decltype(f()) g(); >>> auto x = g(); >> >>> A program containing this translation unit is ill-formed because g is odr-used but not defined, and cannot be defined in any other translation unit because the local class A cannot be named outside this translation unit. >> >> But g could be defined in another translation unit if f is inline or in a >> module interface unit. >> >> So, I think no_linkage_check needs to consider module_has_cmi_p as well as >> vague_linkage_p for relaxed mode. And in no_linkage_error if >> no_linkage_check returns null in relaxed mode, reduce the permerror to a >> pedwarn before C++20, and no diagnostic at all in C++20 and above. >> > > Thanks for the pointers, I've implemented this below. > >>> + if (ctx != NULL_TREE && TREE_PUBLIC (ctx) && module_has_cmi_p ()) >>> + { >>> + /* Ensure that functions in local classes within named modules >>> + have their definitions exported, in case they are (directly >>> + or indirectly) used by an importer. */ >>> + TREE_PUBLIC (decl1) = true; >>> + if (DECL_DECLARED_INLINE_P (decl1)) >>> + comdat_linkage (decl1); >>> + else >>> + mark_needed (decl1); >>> + } >> >> Isn't the inline case handled by the comdat_linkage just above? >> >> Jason >> > > It wasn't, because 'TREE_PUBLIC (decl1)' wasn't yet set. But this means > that it's ended up a lot cleaner to do this in grokfndecl instead then, > which (along with the no_linkage_check changes) means I didn't actually > need to change this code at all. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? OK. > -- >8 -- > > When testing the changes for PR c++/112631 we discovered that currently > we don't emit definitions of block-scope function declarations if > they're not used in the module interface TU, which causes issues if they > are used by importers. > > This patch fixes the handling of no-linkage declarations for C++20. In > particular, a type declared in a function with vague linkage or declared > in a module CMI could potentially be accessible outside its defining TU, > and as such we can't assume that function declarations using that type > can never be defined in another TU. > > A complication with handling this is that we're only strictly interested > in declarations with a module CMI, but when parsing the global module > fragment we don't yet know whether or not this module will have a CMI > until we reach the "export module" line (or not). Since this case is > IFNDR anyway (by [basic.def.odr] p11) we just tentatively assume while > parsing the GMF that this module will have a CMI; once we see (or don't > see) an 'export module' declaration we can commit to that knowledge for > future declarations. > > gcc/cp/ChangeLog: > > * cp-tree.h (module_maybe_has_cmi_p): New function. > * decl.cc (grokfndecl): Mark block-scope functions as public if > they could be visible in other TUs. > * decl2.cc (no_linkage_error): Don't error for declarations that > could be defined in other TUs since C++20. Suppress duplicate > errors from 'check_global_declaration'. > * tree.cc (no_linkage_check): In relaxed mode, don't consider > types in a module CMI to have no linkage. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/linkage-1.C: New test. > * g++.dg/modules/block-decl-3.h: New test. > * g++.dg/modules/block-decl-3_a.C: New test. > * g++.dg/modules/block-decl-3_b.C: New test. > * g++.dg/modules/block-decl-3_c.C: New test. > * g++.dg/modules/linkage-1_a.C: New test. > * g++.dg/modules/linkage-1_b.C: New test. > * g++.dg/modules/linkage-1_c.C: New test. > * g++.dg/modules/linkage-2.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/cp-tree.h | 6 + > gcc/cp/decl.cc | 10 +- > gcc/cp/decl2.cc | 39 ++++- > gcc/cp/tree.cc | 21 ++- > gcc/testsuite/g++.dg/cpp2a/linkage-1.C | 18 ++ > gcc/testsuite/g++.dg/modules/block-decl-3.h | 39 +++++ > gcc/testsuite/g++.dg/modules/block-decl-3_a.C | 157 ++++++++++++++++++ > gcc/testsuite/g++.dg/modules/block-decl-3_b.C | 8 + > gcc/testsuite/g++.dg/modules/block-decl-3_c.C | 30 ++++ > gcc/testsuite/g++.dg/modules/linkage-1_a.C | 15 ++ > gcc/testsuite/g++.dg/modules/linkage-1_b.C | 6 + > gcc/testsuite/g++.dg/modules/linkage-1_c.C | 9 + > gcc/testsuite/g++.dg/modules/linkage-2.C | 26 +++ > 13 files changed, 372 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/linkage-1.C > create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3.h > create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/block-decl-3_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-1_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/linkage-2.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 05913861e06..52d53589e51 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7384,6 +7384,12 @@ inline bool named_module_purview_p () > inline bool named_module_attach_p () > { return named_module_p () && module_attach_p (); } > > +/* We don't know if this TU will have a CMI while parsing the GMF, > + so tentatively assume that it might, for the purpose of determining > + whether no-linkage decls could be used by an importer. */ > +inline bool module_maybe_has_cmi_p () > +{ return module_has_cmi_p () || (named_module_p () && !module_purview_p ()); } > + > /* We're currently exporting declarations. */ > inline bool module_exporting_p () > { return module_kind & MK_EXPORTING; } > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 7a97b867199..65ab64885ff 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -10756,9 +10756,15 @@ grokfndecl (tree ctype, > > /* Members of anonymous types and local classes have no linkage; make > them internal. If a typedef is made later, this will be changed. */ > - if (ctype && (!TREE_PUBLIC (TYPE_MAIN_DECL (ctype)) > - || decl_function_context (TYPE_MAIN_DECL (ctype)))) > + if (ctype && !TREE_PUBLIC (TYPE_MAIN_DECL (ctype))) > publicp = 0; > + else if (ctype && decl_function_context (TYPE_MAIN_DECL (ctype))) > + /* But members of local classes in a module CMI should have their > + definitions exported, in case they are (directly or indirectly) > + used by an importer. We don't just use module_has_cmi_p here > + because for entities in the GMF we don't yet know whether this > + module will have a CMI, so we'll conservatively assume it might. */ > + publicp = module_maybe_has_cmi_p (); > > if (publicp && cxx_dialect == cxx98) > { > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc > index 6c9fd415d40..2562d8aeff6 100644 > --- a/gcc/cp/decl2.cc > +++ b/gcc/cp/decl2.cc > @@ -4696,8 +4696,19 @@ no_linkage_error (tree decl) > bool d = false; > auto_diagnostic_group grp; > if (cxx_dialect >= cxx11) > - d = permerror (DECL_SOURCE_LOCATION (decl), "%q#D, declared using " > - "unnamed type, is used but never defined", decl); > + { > + /* If t is declared in a module CMI, then decl could actually > + be defined in a different TU, so don't warn since C++20. */ > + tree relaxed = no_linkage_check (t, /*relaxed_p=*/true); > + if (relaxed != NULL_TREE) > + d = permerror (DECL_SOURCE_LOCATION (decl), > + "%q#D, declared using an unnamed type, " > + "is used but never defined", decl); > + else if (cxx_dialect < cxx20) > + d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions, > + "%q#D, declared using an unnamed type, " > + "is used but not defined", decl); > + } > else if (DECL_EXTERN_C_P (decl)) > /* Allow this; it's pretty common in C. */; > else if (VAR_P (decl)) > @@ -4716,13 +4727,31 @@ no_linkage_error (tree decl) > inform (DECL_SOURCE_LOCATION (TYPE_NAME (t)), "%q#D does not refer " > "to the unqualified type, so it is not used for linkage", > TYPE_NAME (t)); > + /* Suppress warning from check_global_declaration if needed. */ > + if (d) > + suppress_warning (decl, OPT_Wunused); > } > else if (cxx_dialect >= cxx11) > { > if (VAR_P (decl) || !DECL_PURE_VIRTUAL_P (decl)) > - permerror (DECL_SOURCE_LOCATION (decl), > - "%q#D, declared using local type " > - "%qT, is used but never defined", decl, t); > + { > + /* Similarly for local types in a function with vague linkage or > + defined in a module CMI, then decl could actually be defined > + in a different TU, so don't warn since C++20. */ > + bool d = false; > + tree relaxed = no_linkage_check (t, /*relaxed_p=*/true); > + if (relaxed != NULL_TREE) > + d = permerror (DECL_SOURCE_LOCATION (decl), > + "%q#D, declared using local type " > + "%qT, is used but never defined", decl, t); > + else if (cxx_dialect < cxx20) > + d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions, > + "%q#D, declared using local type " > + "%qT, is used but not defined here", decl, t); > + /* Suppress warning from check_global_declaration if needed. */ > + if (d) > + suppress_warning (decl, OPT_Wunused); > + } > } > else if (VAR_P (decl)) > warning_at (DECL_SOURCE_LOCATION (decl), 0, "type %qT with no linkage " > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > index e75be9a4e66..f1a23ffe817 100644 > --- a/gcc/cp/tree.cc > +++ b/gcc/cp/tree.cc > @@ -2971,7 +2971,8 @@ verify_stmt_tree (tree t) > > /* Check if the type T depends on a type with no linkage and if so, > return it. If RELAXED_P then do not consider a class type declared > - within a vague-linkage function to have no linkage. Remember: > + within a vague-linkage function or in a module CMI to have no linkage, > + since it can still be accessed within a different TU. Remember: > no-linkage is not the same as internal-linkage. */ > > tree > @@ -3012,7 +3013,15 @@ no_linkage_check (tree t, bool relaxed_p) > /* Only treat unnamed types as having no linkage if they're at > namespace scope. This is core issue 966. */ > if (TYPE_UNNAMED_P (t) && TYPE_NAMESPACE_SCOPE_P (t)) > - return t; > + { > + if (relaxed_p > + && TREE_PUBLIC (CP_TYPE_CONTEXT (t)) > + && module_maybe_has_cmi_p ()) > + /* This type could possibly be accessed outside this TU. */ > + return NULL_TREE; > + else > + return t; > + } > > for (r = CP_TYPE_CONTEXT (t); ; ) > { > @@ -3023,10 +3032,12 @@ no_linkage_check (tree t, bool relaxed_p) > return no_linkage_check (TYPE_CONTEXT (t), relaxed_p); > else if (TREE_CODE (r) == FUNCTION_DECL) > { > - if (!relaxed_p || !vague_linkage_p (r)) > - return t; > - else > + if (relaxed_p > + && (vague_linkage_p (r) > + || (TREE_PUBLIC (r) && module_maybe_has_cmi_p ()))) > r = CP_DECL_CONTEXT (r); > + else > + return t; > } > else > break; > diff --git a/gcc/testsuite/g++.dg/cpp2a/linkage-1.C b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C > new file mode 100644 > index 00000000000..888ed6fa5b5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C > @@ -0,0 +1,18 @@ > +// { dg-do compile { target c++11 } } > + > +inline auto f() { > + struct A {}; > + return A{}; > +} > +decltype(f()) a(); // { dg-error "used but not defined" "" { target c++17_down } } > + > +auto g() { > + struct A {}; > + return A{}; > +} > +decltype(g()) b(); // { dg-error "used but never defined" } > + > +int main() { > + a(); > + b(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3.h b/gcc/testsuite/g++.dg/modules/block-decl-3.h > new file mode 100644 > index 00000000000..4b155eb0054 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/block-decl-3.h > @@ -0,0 +1,39 @@ > +// GMF > + > +// Non-inline function definitions in headers are a recipe for ODR violations, > +// but we should probably support that anyway as its not inherently wrong > +// if only ever included into the GMF of a single module. > + > +auto gmf_n_i() { > + struct X { void f() {} }; > + return X{}; > +} > + > +inline auto gmf_i_i() { > + struct X { void f() {} }; > + return X{}; > +} > + > +auto gmf_n_i_i() { > + struct X { > + auto f() { > + struct Y { > + void g() {} > + }; > + return Y{}; > + } > + }; > + return X{}; > +} > + > +inline auto gmf_i_i_i() { > + struct X { > + auto f() { > + struct Y { > + void g() {} > + }; > + return Y{}; > + } > + }; > + return X{}; > +} > diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_a.C b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C > new file mode 100644 > index 00000000000..8cb4dde74d1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C > @@ -0,0 +1,157 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi mod } > + > +// Test that we can link various forms of local class functions. > +// Function names use i=inline, n=non-inline, for each nesting. > + > +module; > +#include "block-decl-3.h" > + > +export module mod; > + > +namespace { > + void internal() {} > +} > + > +// singly-nested > + > +export auto n_n() { > + internal(); > + struct X { void f() { internal(); } }; > + return X{}; > +} > + > +export auto n_i() { > + internal(); > + struct X { inline void f() {} }; > + return X{}; > +} > + > +export inline auto i_n() { > + // `f` is not inline here, so this is OK > + struct X { void f() { internal(); } }; > + return X{}; > +} > + > +export inline auto i_i() { > + struct X { inline void f() {} }; > + return X{}; > +} > + > + > +// doubly nested > + > +export auto n_n_n() { > + struct X { > + auto f() { > + struct Y { > + void g() { internal(); } > + }; > + return Y{}; > + } > + }; > + return X{}; > +} > + > +export auto n_i_n() { > + struct X { > + inline auto f() { > + struct Y { > + void g() { internal(); } > + }; > + return Y{}; > + } > + }; > + return X{}; > +} > + > +export inline auto i_n_i() { > + struct X { > + auto f() { > + struct Y { > + inline void g() {} > + }; > + return Y {}; > + } > + }; > + return X{}; > +} > + > +export inline auto i_i_i() { > + struct X { > + inline auto f() { > + struct Y { > + inline void g() {} > + }; > + return Y{}; > + } > + }; > + return X{}; > +} > + > + > +// multiple types > + > +export auto multi_n_n() { > + struct X { > + void f() { internal(); } > + }; > + struct Y { > + X x; > + }; > + return Y {}; > +} > + > +export auto multi_n_i() { > + struct X { > + inline void f() {} > + }; > + struct Y { > + X x; > + }; > + return Y {}; > +} > + > +export inline auto multi_i_i() { > + struct X { > + inline void f() {} > + }; > + struct Y { > + X x; > + }; > + return Y {}; > +}; > + > + > +// extern "C++" > + > +export extern "C++" auto extern_n_i() { > + struct X { > + void f() {} // implicitly inline > + }; > + return X{}; > +}; > + > +export extern "C++" inline auto extern_i_i() { > + struct X { > + void f() {} > + }; > + return X{}; > +}; > + > + > +// GMF > + > +export using ::gmf_n_i; > +export using ::gmf_i_i; > +export using ::gmf_n_i_i; > +export using ::gmf_i_i_i; > + > + > +// can access from implementation unit > + > +auto only_used_in_impl() { > + struct X { void f() {} }; > + return X{}; > +} > +export void test_from_impl_unit(); > diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_b.C b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C > new file mode 100644 > index 00000000000..bc9b2a213f0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C > @@ -0,0 +1,8 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +module mod; > + > +// Test that we can access (and link) to declarations from the interface > +void test_from_impl_unit() { > + only_used_in_impl().f(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_c.C b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C > new file mode 100644 > index 00000000000..5b39e038327 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C > @@ -0,0 +1,30 @@ > +// { dg-module-do link } > +// { dg-additional-options "-fmodules-ts" } > + > +import mod; > + > +int main() { > + n_n().f(); > + n_i().f(); > + i_n().f(); > + i_i().f(); > + > + n_n_n().f().g(); > + n_i_n().f().g(); > + i_n_i().f().g(); > + i_i_i().f().g(); > + > + multi_n_n().x.f(); > + multi_n_i().x.f(); > + multi_i_i().x.f(); > + > + extern_n_i().f(); > + extern_i_i().f(); > + > + gmf_n_i().f(); > + gmf_i_i().f(); > + gmf_n_i_i().f().g(); > + gmf_i_i_i().f().g(); > + > + test_from_impl_unit(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_a.C b/gcc/testsuite/g++.dg/modules/linkage-1_a.C > new file mode 100644 > index 00000000000..750e31ff347 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-1_a.C > @@ -0,0 +1,15 @@ > +// { dg-additional-options "-fmodules-ts -Wno-error=c++20-extensions" } > +// { dg-module-cmi M } > + > +export module M; > + > +auto f() { > + struct A {}; > + return A{}; > +} > +decltype(f()) g(); // { dg-warning "used but not defined" "" { target c++17_down } } > +export auto x = g(); > + > +struct {} s; > +decltype(s) h(); // { dg-warning "used but not defined" "" { target c++17_down } } > +export auto y = h(); > diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_b.C b/gcc/testsuite/g++.dg/modules/linkage-1_b.C > new file mode 100644 > index 00000000000..f23962d76b7 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-1_b.C > @@ -0,0 +1,6 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +module M; > + > +decltype(f()) g() { return {}; } > +decltype(s) h() { return {}; } > diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_c.C b/gcc/testsuite/g++.dg/modules/linkage-1_c.C > new file mode 100644 > index 00000000000..f1406b99032 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-1_c.C > @@ -0,0 +1,9 @@ > +// { dg-module-do link } > +// { dg-additional-options "-fmodules-ts" } > + > +import M; > + > +int main() { > + auto a = x; > + auto b = y; > +} > diff --git a/gcc/testsuite/g++.dg/modules/linkage-2.C b/gcc/testsuite/g++.dg/modules/linkage-2.C > new file mode 100644 > index 00000000000..eb4d7b051af > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/linkage-2.C > @@ -0,0 +1,26 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi !M } > + > +export module M; > + > +// Same as a linkage-1 except within an anonymous namespace; > +// now these declarations cannot possibly be defined outside this TU, > +// so we should error. > + > +namespace { > + auto f() { > + struct A {}; > + return A{}; > + } > + decltype(f()) g(); // { dg-error "used but never defined" } > + > + struct {} s; > + decltype(s) h(); // { dg-error "used but never defined" } > +} > + > +export void use() { > + g(); > + h(); > +} > + > +// { dg-prune-output "not writing module" }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 05913861e06..52d53589e51 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7384,6 +7384,12 @@ inline bool named_module_purview_p () inline bool named_module_attach_p () { return named_module_p () && module_attach_p (); } +/* We don't know if this TU will have a CMI while parsing the GMF, + so tentatively assume that it might, for the purpose of determining + whether no-linkage decls could be used by an importer. */ +inline bool module_maybe_has_cmi_p () +{ return module_has_cmi_p () || (named_module_p () && !module_purview_p ()); } + /* We're currently exporting declarations. */ inline bool module_exporting_p () { return module_kind & MK_EXPORTING; } diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 7a97b867199..65ab64885ff 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -10756,9 +10756,15 @@ grokfndecl (tree ctype, /* Members of anonymous types and local classes have no linkage; make them internal. If a typedef is made later, this will be changed. */ - if (ctype && (!TREE_PUBLIC (TYPE_MAIN_DECL (ctype)) - || decl_function_context (TYPE_MAIN_DECL (ctype)))) + if (ctype && !TREE_PUBLIC (TYPE_MAIN_DECL (ctype))) publicp = 0; + else if (ctype && decl_function_context (TYPE_MAIN_DECL (ctype))) + /* But members of local classes in a module CMI should have their + definitions exported, in case they are (directly or indirectly) + used by an importer. We don't just use module_has_cmi_p here + because for entities in the GMF we don't yet know whether this + module will have a CMI, so we'll conservatively assume it might. */ + publicp = module_maybe_has_cmi_p (); if (publicp && cxx_dialect == cxx98) { diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index 6c9fd415d40..2562d8aeff6 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -4696,8 +4696,19 @@ no_linkage_error (tree decl) bool d = false; auto_diagnostic_group grp; if (cxx_dialect >= cxx11) - d = permerror (DECL_SOURCE_LOCATION (decl), "%q#D, declared using " - "unnamed type, is used but never defined", decl); + { + /* If t is declared in a module CMI, then decl could actually + be defined in a different TU, so don't warn since C++20. */ + tree relaxed = no_linkage_check (t, /*relaxed_p=*/true); + if (relaxed != NULL_TREE) + d = permerror (DECL_SOURCE_LOCATION (decl), + "%q#D, declared using an unnamed type, " + "is used but never defined", decl); + else if (cxx_dialect < cxx20) + d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions, + "%q#D, declared using an unnamed type, " + "is used but not defined", decl); + } else if (DECL_EXTERN_C_P (decl)) /* Allow this; it's pretty common in C. */; else if (VAR_P (decl)) @@ -4716,13 +4727,31 @@ no_linkage_error (tree decl) inform (DECL_SOURCE_LOCATION (TYPE_NAME (t)), "%q#D does not refer " "to the unqualified type, so it is not used for linkage", TYPE_NAME (t)); + /* Suppress warning from check_global_declaration if needed. */ + if (d) + suppress_warning (decl, OPT_Wunused); } else if (cxx_dialect >= cxx11) { if (VAR_P (decl) || !DECL_PURE_VIRTUAL_P (decl)) - permerror (DECL_SOURCE_LOCATION (decl), - "%q#D, declared using local type " - "%qT, is used but never defined", decl, t); + { + /* Similarly for local types in a function with vague linkage or + defined in a module CMI, then decl could actually be defined + in a different TU, so don't warn since C++20. */ + bool d = false; + tree relaxed = no_linkage_check (t, /*relaxed_p=*/true); + if (relaxed != NULL_TREE) + d = permerror (DECL_SOURCE_LOCATION (decl), + "%q#D, declared using local type " + "%qT, is used but never defined", decl, t); + else if (cxx_dialect < cxx20) + d = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wc__20_extensions, + "%q#D, declared using local type " + "%qT, is used but not defined here", decl, t); + /* Suppress warning from check_global_declaration if needed. */ + if (d) + suppress_warning (decl, OPT_Wunused); + } } else if (VAR_P (decl)) warning_at (DECL_SOURCE_LOCATION (decl), 0, "type %qT with no linkage " diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index e75be9a4e66..f1a23ffe817 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -2971,7 +2971,8 @@ verify_stmt_tree (tree t) /* Check if the type T depends on a type with no linkage and if so, return it. If RELAXED_P then do not consider a class type declared - within a vague-linkage function to have no linkage. Remember: + within a vague-linkage function or in a module CMI to have no linkage, + since it can still be accessed within a different TU. Remember: no-linkage is not the same as internal-linkage. */ tree @@ -3012,7 +3013,15 @@ no_linkage_check (tree t, bool relaxed_p) /* Only treat unnamed types as having no linkage if they're at namespace scope. This is core issue 966. */ if (TYPE_UNNAMED_P (t) && TYPE_NAMESPACE_SCOPE_P (t)) - return t; + { + if (relaxed_p + && TREE_PUBLIC (CP_TYPE_CONTEXT (t)) + && module_maybe_has_cmi_p ()) + /* This type could possibly be accessed outside this TU. */ + return NULL_TREE; + else + return t; + } for (r = CP_TYPE_CONTEXT (t); ; ) { @@ -3023,10 +3032,12 @@ no_linkage_check (tree t, bool relaxed_p) return no_linkage_check (TYPE_CONTEXT (t), relaxed_p); else if (TREE_CODE (r) == FUNCTION_DECL) { - if (!relaxed_p || !vague_linkage_p (r)) - return t; - else + if (relaxed_p + && (vague_linkage_p (r) + || (TREE_PUBLIC (r) && module_maybe_has_cmi_p ()))) r = CP_DECL_CONTEXT (r); + else + return t; } else break; diff --git a/gcc/testsuite/g++.dg/cpp2a/linkage-1.C b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C new file mode 100644 index 00000000000..888ed6fa5b5 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/linkage-1.C @@ -0,0 +1,18 @@ +// { dg-do compile { target c++11 } } + +inline auto f() { + struct A {}; + return A{}; +} +decltype(f()) a(); // { dg-error "used but not defined" "" { target c++17_down } } + +auto g() { + struct A {}; + return A{}; +} +decltype(g()) b(); // { dg-error "used but never defined" } + +int main() { + a(); + b(); +} diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3.h b/gcc/testsuite/g++.dg/modules/block-decl-3.h new file mode 100644 index 00000000000..4b155eb0054 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/block-decl-3.h @@ -0,0 +1,39 @@ +// GMF + +// Non-inline function definitions in headers are a recipe for ODR violations, +// but we should probably support that anyway as its not inherently wrong +// if only ever included into the GMF of a single module. + +auto gmf_n_i() { + struct X { void f() {} }; + return X{}; +} + +inline auto gmf_i_i() { + struct X { void f() {} }; + return X{}; +} + +auto gmf_n_i_i() { + struct X { + auto f() { + struct Y { + void g() {} + }; + return Y{}; + } + }; + return X{}; +} + +inline auto gmf_i_i_i() { + struct X { + auto f() { + struct Y { + void g() {} + }; + return Y{}; + } + }; + return X{}; +} diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_a.C b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C new file mode 100644 index 00000000000..8cb4dde74d1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_a.C @@ -0,0 +1,157 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi mod } + +// Test that we can link various forms of local class functions. +// Function names use i=inline, n=non-inline, for each nesting. + +module; +#include "block-decl-3.h" + +export module mod; + +namespace { + void internal() {} +} + +// singly-nested + +export auto n_n() { + internal(); + struct X { void f() { internal(); } }; + return X{}; +} + +export auto n_i() { + internal(); + struct X { inline void f() {} }; + return X{}; +} + +export inline auto i_n() { + // `f` is not inline here, so this is OK + struct X { void f() { internal(); } }; + return X{}; +} + +export inline auto i_i() { + struct X { inline void f() {} }; + return X{}; +} + + +// doubly nested + +export auto n_n_n() { + struct X { + auto f() { + struct Y { + void g() { internal(); } + }; + return Y{}; + } + }; + return X{}; +} + +export auto n_i_n() { + struct X { + inline auto f() { + struct Y { + void g() { internal(); } + }; + return Y{}; + } + }; + return X{}; +} + +export inline auto i_n_i() { + struct X { + auto f() { + struct Y { + inline void g() {} + }; + return Y {}; + } + }; + return X{}; +} + +export inline auto i_i_i() { + struct X { + inline auto f() { + struct Y { + inline void g() {} + }; + return Y{}; + } + }; + return X{}; +} + + +// multiple types + +export auto multi_n_n() { + struct X { + void f() { internal(); } + }; + struct Y { + X x; + }; + return Y {}; +} + +export auto multi_n_i() { + struct X { + inline void f() {} + }; + struct Y { + X x; + }; + return Y {}; +} + +export inline auto multi_i_i() { + struct X { + inline void f() {} + }; + struct Y { + X x; + }; + return Y {}; +}; + + +// extern "C++" + +export extern "C++" auto extern_n_i() { + struct X { + void f() {} // implicitly inline + }; + return X{}; +}; + +export extern "C++" inline auto extern_i_i() { + struct X { + void f() {} + }; + return X{}; +}; + + +// GMF + +export using ::gmf_n_i; +export using ::gmf_i_i; +export using ::gmf_n_i_i; +export using ::gmf_i_i_i; + + +// can access from implementation unit + +auto only_used_in_impl() { + struct X { void f() {} }; + return X{}; +} +export void test_from_impl_unit(); diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_b.C b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C new file mode 100644 index 00000000000..bc9b2a213f0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_b.C @@ -0,0 +1,8 @@ +// { dg-additional-options "-fmodules-ts" } + +module mod; + +// Test that we can access (and link) to declarations from the interface +void test_from_impl_unit() { + only_used_in_impl().f(); +} diff --git a/gcc/testsuite/g++.dg/modules/block-decl-3_c.C b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C new file mode 100644 index 00000000000..5b39e038327 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/block-decl-3_c.C @@ -0,0 +1,30 @@ +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts" } + +import mod; + +int main() { + n_n().f(); + n_i().f(); + i_n().f(); + i_i().f(); + + n_n_n().f().g(); + n_i_n().f().g(); + i_n_i().f().g(); + i_i_i().f().g(); + + multi_n_n().x.f(); + multi_n_i().x.f(); + multi_i_i().x.f(); + + extern_n_i().f(); + extern_i_i().f(); + + gmf_n_i().f(); + gmf_i_i().f(); + gmf_n_i_i().f().g(); + gmf_i_i_i().f().g(); + + test_from_impl_unit(); +} diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_a.C b/gcc/testsuite/g++.dg/modules/linkage-1_a.C new file mode 100644 index 00000000000..750e31ff347 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-1_a.C @@ -0,0 +1,15 @@ +// { dg-additional-options "-fmodules-ts -Wno-error=c++20-extensions" } +// { dg-module-cmi M } + +export module M; + +auto f() { + struct A {}; + return A{}; +} +decltype(f()) g(); // { dg-warning "used but not defined" "" { target c++17_down } } +export auto x = g(); + +struct {} s; +decltype(s) h(); // { dg-warning "used but not defined" "" { target c++17_down } } +export auto y = h(); diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_b.C b/gcc/testsuite/g++.dg/modules/linkage-1_b.C new file mode 100644 index 00000000000..f23962d76b7 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-1_b.C @@ -0,0 +1,6 @@ +// { dg-additional-options "-fmodules-ts" } + +module M; + +decltype(f()) g() { return {}; } +decltype(s) h() { return {}; } diff --git a/gcc/testsuite/g++.dg/modules/linkage-1_c.C b/gcc/testsuite/g++.dg/modules/linkage-1_c.C new file mode 100644 index 00000000000..f1406b99032 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-1_c.C @@ -0,0 +1,9 @@ +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts" } + +import M; + +int main() { + auto a = x; + auto b = y; +} diff --git a/gcc/testsuite/g++.dg/modules/linkage-2.C b/gcc/testsuite/g++.dg/modules/linkage-2.C new file mode 100644 index 00000000000..eb4d7b051af --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/linkage-2.C @@ -0,0 +1,26 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi !M } + +export module M; + +// Same as a linkage-1 except within an anonymous namespace; +// now these declarations cannot possibly be defined outside this TU, +// so we should error. + +namespace { + auto f() { + struct A {}; + return A{}; + } + decltype(f()) g(); // { dg-error "used but never defined" } + + struct {} s; + decltype(s) h(); // { dg-error "used but never defined" } +} + +export void use() { + g(); + h(); +} + +// { dg-prune-output "not writing module" }