On Fri, 26 Jan 2024, Nathaniel Shead wrote:
> This patch just adds enough of the fields from 'function' to fix the ICE
> in the linked PR. I suppose there might be more fields from this type
> that should be propagated, but I don't know enough to find out which
> they might be yet, since a lot of them seem to be only set after
> gimplification.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>
> -- >8 --
>
> Currently the DECL_STRUCT_FUNCTION for a declaration is always
> reconstructed from scratch. This causes issues though, as some fields
> used by other parts of the compiler (in this case, specifically
> 'function_{start,end}_locus') are then not correctly initialised. This
> patch makes sure that these fields are also read and written.
LGTM
>
> PR c++/113580
>
> gcc/cp/ChangeLog:
>
> * module.cc (struct post_process_data): Create.
> (trees_in::post_decls): Use.
> (trees_in::post_process): Return entire vector at once.
> Change overload to take post_process_data instead of tree.
> (trees_out::write_function_def): Write needed flags from
> DECL_STRUCT_FUNCTION.
> (trees_in::read_function_def): Read them and pass to
> post_process.
> (module_state::read_cluster): Write flags into cfun.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/pr113580_a.C: New test.
> * g++.dg/modules/pr113580_b.C: New test.
>
> Signed-off-by: Nathaniel Shead
> ---
> gcc/cp/module.cc | 47 ++-
> gcc/testsuite/g++.dg/modules/pr113580_a.C | 10 +
> gcc/testsuite/g++.dg/modules/pr113580_b.C | 10 +
> 3 files changed, 58 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/pr113580_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/pr113580_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 6176801b7a7..840c7ef6dab 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -2837,6 +2837,13 @@ typedef hash_mapsimple_hashmap_traits,uintptr_t> >
> duplicate_hash_map;
>
> +/* Data needed for post-processing. */
> +struct post_process_data {
> + tree decl;
> + location_t start_locus;
> + location_t end_locus;
> +};
> +
> /* Tree stream reader. Note that reading a stream doesn't mark the
> read trees with TREE_VISITED. Thus it's quite safe to have
> multiple concurrent readers. Which is good, because lazy
> @@ -2848,7 +2855,7 @@ private:
>module_state *state; /* Module being imported. */
>vec back_refs; /* Back references. */
>duplicate_hash_map *duplicates;/* Map from existings to duplicate. */
> - vec post_decls; /* Decls to post process. */
> + vec post_decls; /* Decls to post process. */
>unsigned unused; /* Inhibit any interior TREE_USED
> marking. */
>
> @@ -2945,16 +2952,16 @@ public:
>tree odr_duplicate (tree decl, bool has_defn);
>
> public:
> - /* Return the next decl to postprocess, or NULL. */
> - tree post_process ()
> + /* Return the decls to postprocess. */
> + const vec& post_process ()
>{
> -return post_decls.length () ? post_decls.pop () : NULL_TREE;
> +return post_decls;
>}
> private:
> - /* Register DECL for postprocessing. */
> - void post_process (tree decl)
> + /* Register DATA for postprocessing. */
> + void post_process (post_process_data data)
>{
> -post_decls.safe_push (decl);
> +post_decls.safe_push (data);
>}
>
> private:
> @@ -11667,15 +11674,25 @@ trees_out::write_function_def (tree decl)
>tree_node (cexpr->body);
> }
>
> + function* f = DECL_STRUCT_FUNCTION (decl);
> +
>if (streaming_p ())
> {
>unsigned flags = 0;
>
> + if (f)
> + flags |= 2;
>if (DECL_NOT_REALLY_EXTERN (decl))
> flags |= 1;
>
>u (flags);
> }
> +
> + if (state && f)
> +{
> + state->write_location (*this, f->function_start_locus);
> + state->write_location (*this, f->function_end_locus);
> +}
> }
>
> void
> @@ -11692,6 +11709,8 @@ trees_in::read_function_def (tree decl, tree
> maybe_template)
>tree saved = tree_node ();
>tree context = tree_node ();
>constexpr_fundef cexpr;
> + post_process_data pdata {};
> + pdata.decl = maybe_template;
>
>tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl));
>bool installing = maybe_dup && !DECL_SAVED_TREE (decl);
> @@ -11708,6 +11727,12 @@ trees_in::read_function_def (tree decl, tree
> maybe_template)
>
>unsigned flags = u ();
>
> + if (flags & 2)
> +{
> + pdata.start_locus = state->read_location (*this);
> + pdata.end_locus = state->read_location (*this);
> +}
> +
>if (get_overrun ())
> return NULL_TREE;
>
> @@ -11722,7 +11747,7 @@ trees_in::read_function_def (tree decl, tree
> maybe_template)
>