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?
> > +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?
>
> Jason
>
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). So I think this example is OK, but this example is not:
namespace {
auto x() {
struct X { void f() {} };
return X{};
}
}
export auto illegal() {
return x();
}
Which we correctly complain about already:
error: 'struct {anonymous}::x()::X' references internal linkage entity 'auto
{anonymous}::x()'
6 | struct X { void f() {} };
| ^
Nathaniel
