On 10/2/25 2:13 PM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
This approach does have some downsides, in that this early folding does
potentially impact later-running diagnostic messages and overall could
make things more difficult to understand; I'm not sure what a good
alternative approach would be though. Any thoughts welcome.
For those reasons, as much as possible we've tried to defer folding
until cp_fold_function, called from finish_function. It seems the
problem here is that folding happens after the call to
maybe_save_constexpr_fundef, so perhaps we need an earlier pass to do
only non-ODR-use folding?
A much more involved option might be to adopt C_MAYBE_CONST_EXPR from
the C front-end to retain both folded and unfolded expressions, and then
discard the unfolded expression in module streaming if it refers to a
TU-local value. Perhaps this could also replace the cv_cache.
I'm also unsure about the OpenMP changes: they're required here because
otherwise it looks like things get confused by the reduction variable
not matching the expressions that end up getting used in the loop body,
but I'm not familiar enough with OpenMP to know if this might have other
ill effects.
-- >8 --
[basic.link] p14.4 says that a declaration naming a TU-local entity is
an exposure, ignoring "any reference to a non-volatile const object or
reference with internal or no linkage initialized with a constant
expression that is not an odr-use".
To implement this, we cannot stream these entities but must fold them
into their underlying values beforehand. This patch does this within
mark_use as a central location, rather than having to reimplement the
ODR-use logic within modules streaming, and doing this multiple times
for e.g. saved constexpr function bodies. This approach also benefits
from all the existing non-modules testcases for the change.
We implement this as an early walk in mark_use. We don't want to just
piggyback off the recursion done here because we need to fold the entire
member access so we don't leave references to TU-local types around,
so the patch adds an early walk to find candidate expressions. To
prevent unnecessarily repeating this the patch also adds a parameter to
indicate when in a recursive call this would be interesting to attempt
again.
This early folding does break a few assumptions elsewhere in the
frontend that need to be adjusted. We need to wrap some more
expressions with location wrappers to not regress diagnostic quality
after folding, and then handle those new location wrappers. We also
need to prevent folding arithmetic on variables with nullptr value so
that constexpr handling can error later, and prevent folding of
dependent static_assertion conditions so that we can keep the original
values around for diagnose_failing_condition. We also need to update
OpenMP handling so that reductions refer to the underlying decl rather
than a reference, in case the reference gets removed later.
This fix leaves out a few cases that are trickier to handle:
- bit fields have special handling and so can't be completely folded
out, as we need to remember in some cases that this was a bit-field
access and a plain INTEGER_CST doesn't have the space for that.
When does this happen with a non-ODR-use?
- variables of type pointer-to-member function have a DECL_INITIAL with
a CONSTRUCTOR that has a different type to the containing variable
(which is a typedef-decl). I wasn't able to find a good way to handle
this and I expect it to be a rare case so I'm leaving it for later.
- Templates when building the tree don't call mark_rvalue_use in all
scenarios, and even in places where they do (e.g. binop handling)
they throw away the result and instead call a 'build_min_nt_*'
function with the original operands. This seems complex and
intrusive to fix, so I'll leave that for a later patch as well.
I think we should not try to handle this case; in general we don't know
whether something is an ODR-use in a template until instantiation time.
This is also why a generic lambda captures more than an equivalent
non-generic lambda.
+ /* Don't count a folded use of the value as a use for the purposes
+ of warnings. (Ideally it shouldn't be a DECL_ODR_USE either,
+ but it's too late to put the cat back in the bag...) */
+ bool used = TREE_USED (var);
+ tree t = maybe_constant_value (expr);
+ if (!used)
+ TREE_USED (var) = false;
It seems like a bug that folding changes flags like
TREE_USED/DECL_READ_P. Jakub's r16-2258 tried to reduce that (search
for clear_exp_read in cp-gimplify) but a more comprehensive fix would be
nice. Maybe just adding TREE_USED to the DECL_READ_P handling there is
enough.
Jason
