On Fri, Oct 03, 2025 at 05:28:59PM +0100, Jason Merrill wrote:
> 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?
>
Right. I'd previously dismissed this approach as it appeared
fundamentally incompatible with supporting this in templates, but as you
point out below that's impossible in general so this does some more
reasonable.
It's just not constexpr functions, it turns out even regular inline
functions we don't fold away all non-ODR-used constants in some cases
(this is PR c++/199097), but I guess we're just missing some folding
that we could be doing; I'll need to look further.
> 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 like this idea a lot, but it doesn't seem feasible to do before end of
stage 1. I might have a play with it for GCC17 timeframe, depending on
what else I end up working on and how much time I have.
> > 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?
>
One example is in g++.dg/expr/bitfield13.C. A stripped-down example is:
struct flags {
enum field { f0, f1, no_field };
field b0 : 4;
};
int main() {
constexpr flags f{ flags::f0 };
bool b = f.b0;
}
This is not an ODR-use of 'f' or of 'f.b0', but with this patch we
failed a checking assertion in maybe_adjust_type_name:
gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p (etype, type)
|| is_bitfield_expr_with_lowered_type (expr)
|| seen_error ());
because we no longer satisfy 'is_bitfield_expr_with_lowered_type'.
I wasn't sure if just removing this assertion could possibly expose
other issues with this change in behaviour down the line so opted to
just not include bitfields. By folding late we should avoid this issue
entirely though.
Nathaniel
> > - 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
>
