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.
+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.
+ 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
