On Fri, 1 Mar 2024, Jason Merrill wrote: > On 3/1/24 10:32, Jason Merrill wrote: > > On 3/1/24 10:00, Patrick Palka wrote: > > > 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 > > > > Yes, I believe that's what modules.cc refers to as a "voldemort". > > > > But for non-voldemort local types, the declaration of the function doesn't > > depend on them, only the definition. Why doesn't streaming them in the > > definition work properly? > > And does your 99426 patch address that problem?
I don't think so, that patch should only affect declaration merging (of a streamed-in local type with the corresponding in-TU local type after their containing function is merged). > > 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. Unfortunately this approached caused a boostrap failure with Ada: raised STORAGE_ERROR : stack overflow or erroneous memory access The patch was --- a/gcc/tree-inline.cc +++ b/gcc/tree-inline.cc @@ -725,6 +725,9 @@ can_be_nonlocal (tree decl, copy_body_data *id) if (TREE_CODE (decl) == FUNCTION_DECL) return true; + if (TREE_CODE (decl) == TYPE_DECL || TREE_CODE (decl) == CONST_DECL) + return true; + /* Local static vars must be non-local or we get multiple declaration problems. */ if (VAR_P (decl) && !auto_var_in_fn_p (decl, id->src_fn)) > > Jason > >