On Fri, Dec 19, 2025 at 10:37:19PM +0700, Jason Merrill wrote:
> On 12/19/25 7:53 PM, Nathaniel Shead wrote:
> > On Fri, Dec 19, 2025 at 12:29:04PM +0700, Jason Merrill wrote:
> > > On 12/4/25 9:40 PM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > 
> > > > -- >8 --
> > > > 
> > > > As the PR rightly points out, a lambda is not really a declaration in
> > > > and of itself by the standard, and so a lambda only used in a context
> > > > where exposures are ignored should not itself cause an error.
> > > > 
> > > > This patch implements this by way of a new flag set on deps that are
> > > > first found in an ignored context.  This flag gets cleared if we ever
> > > > see the dep in a context where exposures are not ignored.  Then, while
> > > > walking a declaration with this flag set, we re-establish an ignored
> > > > context.  This is done for all decls (not just lambdas) to handle
> > > > block-scope classes as well.
> > > > 
> > > > Additionally, we prevent walking of attached declarations for a
> > > > DECL_MODULE_KEYED_DECLS_P entity during dependency gathering, so that we
> > > > don't think we've seen the decl at this point.  This means we may not
> > > > have an appropriate entity to stream for this walk; to prevent any
> > > > potential issues with merging we stream a NULL_TREE 'hole' in the vector
> > > > and handle this carefully on import.
> > > > 
> > > > This causes a small amount of testsuite fallout because we no longer
> > > > diagnose errors we really should.  Because our ABI for inline variables
> > > > with dynamic initialization is to just do the initialization in the
> > > > module's initializer function (and importers only perform the static
> > > > initialization) we don't bother to walk the definition of inline
> > > > variables containing lambdas and so don't see the exposures, despite
> > > > this being a non-ignored context.
> > > 
> > > Hmm, looking back at the standard it seems that any variable initializer 
> > > is
> > > an ignored context under [basic.link]/14.2, inline only matters for
> > > functions.  So this actually sounds like an improvement, not a regression.
> > 
> > Right; this is related to r15-9136 (PR119551) where we started
> > complaining about TU-local entities in inline variables because strictly
> > following the standard here is worse (wrong-code).  But by standard we
> > shouldn't be complaining about any of these scenarios to start with.
> > 
> > This patch is the opposite, though, about us complaining about TU-local
> > entities in the body of a lambda within a non-inline variable definition
> > (i.e. rejects-valid), which I think is still a bug worth fixing.  But
> > yes, it's not a regression (we always complained about this case), so if
> > you'd prefer to leave it till GCC17 I'm OK with that.
> 
> The patch looks good to me, I'm just questioning the commit message.
> 
> I meant that the testsuite fallout you mention isn't a problem, because from
> what you are saying about only doing the initialization in the interface
> unit it sounds like we ended up with option 1 for PR119551 after all.
> 
> So perhaps we should revert the changes for option 2?
> 

Right, thanks; sorry for the confusion.

Currently we do option 1 for dynamic initialisation (the initialisation
is done in the module's initialiser function and importers rely on that
function getting called to perform it), but static initialisation is
done by each caller.  I think this is still needed unless we do further
work since statically initialised inline variables will only be emitted
in TUs that ODR-use them, which is not necessarily the module interface,
so importers still need the definition to be able to emit it correctly
if needed.

I'm sure we could adjust this but I'm not really sure how to yet; in
particular I'd like to avoid having to just emit all vague linkage
variables in a module interface even if they are a discarded declaration
from the GMF, and I'm not yet sure how to do that well.

I've adjusted the commit message locally to the following, OK for trunk?

--

As the PR rightly points out, a lambda is not really a declaration in
and of itself by the standard, and so a lambda only used in a context
where exposures are ignored should not itself cause an error.

This patch implements this by way of a new flag set on deps that are
first found in an ignored context.  This flag gets cleared if we ever
see the dep in a context where exposures are not ignored.  Then, while
walking a declaration with this flag set, we re-establish an ignored
context.  This is done for all decls (not just lambdas) to handle
block-scope classes as well.

Additionally, we prevent walking of attached declarations for a
DECL_MODULE_KEYED_DECLS_P entity during dependency gathering, so that we
don't think we've seen the decl at this point.  This means we may not
have an appropriate entity to stream for this walk; to prevent any
potential issues with merging we stream a NULL_TREE 'hole' in the vector
and handle this carefully on import.

This requires a small amount of testsuite adjustment because we no
longer diagnose errors we used to.  Because our ABI for inline variables
with dynamic initialization is to just do the initialization in the
module's initializer function (and importers only perform the static
initialization) we don't bother to walk the definition of inline
variables containing lambdas and so don't see the exposures, despite
us considering TU-local entities in static initializers of inline
variables being exposures (see PR c++/119551).  This is legal by the
current wording of the standard, which does not consider the definition
of any variable to be an exposure (even an inline one).

Reply via email to