On Fri, 1 Mar 2024, Jason Merrill wrote:
> On 2/29/24 15:56, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk?
> >
> > -- >8 --
> >
> > For local enums defined in a non-template function or a function template
> > instantiation it seems we neglect to make the function depend on the enum
> > definition, which ultimately causes streaming to fail due to the enum
> > definition not being streamed before uses of its enumerators are streamed,
> > as far as I can tell.
>
> I would think that the function doesn't need to depend on the local enum in
> order for the local enum to be streamed before the use of the enumerator,
> which comes after the definition of the enum in the function body?
>
> Why isn't streaming the body of the function outputting the enum definition
> before the use of the enumerator?
IIUC (based on observing the behavior for local classes) streaming the
definition of a local class/enum as part of the function definition is
what we want to avoid; we want to treat a local type definition as a
logically separate definition and stream it separately (similar
to class defns vs member defns I guess). And by not registering a dependency
between the function and the local enum, we end up never streaming out
the local enum definition separately and instead stream it out as part
of the function definition (accidentally) which we then can't stream in
properly.
Perhaps the motivation for treating local type definitions as logically
separate from the function definition is because they can leak out of a
function with a deduced return type:
auto f() {
struct A { };
return A();
}
using type = decltype(f()); // refers directly to f()::A
It's also consistent with templated local types getting their own
TEMPLATE_DECL (which Nathan revisited in r11-4529-g9703b8d98c116e).
>
> > This was nearly enough to make things work, except we now ran into
> > issues with the local TYPE/CONST_DECL copies when streaming the
> > constexpr version of a function body. It occurred to me that we don't
> > need to make copies of local types when copying a constexpr function
> > body; only VAR_DECLs etc need to be copied for sake of recursive
> > constexpr calls. So this patch adjusts copy_fn accordingly.
>
> Maybe adjust can_be_nonlocal instead? It seems unnecessary in general to
> remap types and enumerators for inlining.
That seems to work nicely too. I'm testing a patch to that effect with
all front ends and will report back.
>
> Jason
>
>
