* [PATCH] c++/modules: Stream additional fields for DECL_STRUCT_FUNCTION [PR113580]
@ 2024-01-26 11:28 Nathaniel Shead
2024-01-26 15:49 ` Patrick Palka
0 siblings, 1 reply; 3+ messages in thread
From: Nathaniel Shead @ 2024-01-26 11:28 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell
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.
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 <nathanieloshead@gmail.com>
---
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_map<tree,uintptr_t,
simple_hashmap_traits<nodel_ptr_hash<tree_node>,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<tree> back_refs; /* Back references. */
duplicate_hash_map *duplicates; /* Map from existings to duplicate. */
- vec<tree> post_decls; /* Decls to post process. */
+ vec<post_process_data> 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_data>& 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)
SET_DECL_FRIEND_CONTEXT (decl, context);
if (cexpr.decl)
register_constexpr_fundef (cexpr);
- post_process (maybe_template);
+ post_process (pdata);
}
else if (maybe_dup)
{
@@ -15100,8 +15125,10 @@ module_state::read_cluster (unsigned snum)
push_function_context does too much work. */
tree old_cfd = current_function_decl;
struct function *old_cfun = cfun;
- while (tree decl = sec.post_process ())
+ for (const post_process_data& pdata : sec.post_process ())
{
+ tree decl = pdata.decl;
+
bool abstract = false;
if (TREE_CODE (decl) == TEMPLATE_DECL)
{
@@ -15113,6 +15140,8 @@ module_state::read_cluster (unsigned snum)
allocate_struct_function (decl, abstract);
cfun->language = ggc_cleared_alloc<language_function> ();
cfun->language->base.x_stmt_tree.stmts_are_full_exprs_p = 1;
+ cfun->function_start_locus = pdata.start_locus;
+ cfun->function_end_locus = pdata.end_locus;
if (abstract)
;
diff --git a/gcc/testsuite/g++.dg/modules/pr113580_a.C b/gcc/testsuite/g++.dg/modules/pr113580_a.C
new file mode 100644
index 00000000000..954333f5038
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr113580_a.C
@@ -0,0 +1,10 @@
+// PR c++/113580
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+export {
+ template <typename T>
+ void fun(T x) {}
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr113580_b.C b/gcc/testsuite/g++.dg/modules/pr113580_b.C
new file mode 100644
index 00000000000..a72cd750c72
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr113580_b.C
@@ -0,0 +1,10 @@
+// PR c++/113580
+// { dg-additional-options "-fmodules-ts -Wunused-parameter" }
+
+import A;
+
+int main() {
+ fun(42); // { dg-message "required from here" }
+}
+
+// { dg-warning "unused parameter" "" { target *-*-* } 0 }
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++/modules: Stream additional fields for DECL_STRUCT_FUNCTION [PR113580]
2024-01-26 11:28 [PATCH] c++/modules: Stream additional fields for DECL_STRUCT_FUNCTION [PR113580] Nathaniel Shead
@ 2024-01-26 15:49 ` Patrick Palka
2024-01-26 19:54 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Patrick Palka @ 2024-01-26 15:49 UTC (permalink / raw)
To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell
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 <nathanieloshead@gmail.com>
> ---
> 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_map<tree,uintptr_t,
> simple_hashmap_traits<nodel_ptr_hash<tree_node>,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<tree> back_refs; /* Back references. */
> duplicate_hash_map *duplicates; /* Map from existings to duplicate. */
> - vec<tree> post_decls; /* Decls to post process. */
> + vec<post_process_data> 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_data>& 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)
> SET_DECL_FRIEND_CONTEXT (decl, context);
> if (cexpr.decl)
> register_constexpr_fundef (cexpr);
> - post_process (maybe_template);
> + post_process (pdata);
> }
> else if (maybe_dup)
> {
> @@ -15100,8 +15125,10 @@ module_state::read_cluster (unsigned snum)
> push_function_context does too much work. */
> tree old_cfd = current_function_decl;
> struct function *old_cfun = cfun;
> - while (tree decl = sec.post_process ())
> + for (const post_process_data& pdata : sec.post_process ())
> {
> + tree decl = pdata.decl;
> +
> bool abstract = false;
> if (TREE_CODE (decl) == TEMPLATE_DECL)
> {
> @@ -15113,6 +15140,8 @@ module_state::read_cluster (unsigned snum)
> allocate_struct_function (decl, abstract);
> cfun->language = ggc_cleared_alloc<language_function> ();
> cfun->language->base.x_stmt_tree.stmts_are_full_exprs_p = 1;
> + cfun->function_start_locus = pdata.start_locus;
> + cfun->function_end_locus = pdata.end_locus;
>
> if (abstract)
> ;
> diff --git a/gcc/testsuite/g++.dg/modules/pr113580_a.C b/gcc/testsuite/g++.dg/modules/pr113580_a.C
> new file mode 100644
> index 00000000000..954333f5038
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr113580_a.C
> @@ -0,0 +1,10 @@
> +// PR c++/113580
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi A }
> +
> +export module A;
> +
> +export {
> + template <typename T>
> + void fun(T x) {}
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/pr113580_b.C b/gcc/testsuite/g++.dg/modules/pr113580_b.C
> new file mode 100644
> index 00000000000..a72cd750c72
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr113580_b.C
> @@ -0,0 +1,10 @@
> +// PR c++/113580
> +// { dg-additional-options "-fmodules-ts -Wunused-parameter" }
> +
> +import A;
> +
> +int main() {
> + fun(42); // { dg-message "required from here" }
> +}
> +
> +// { dg-warning "unused parameter" "" { target *-*-* } 0 }
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++/modules: Stream additional fields for DECL_STRUCT_FUNCTION [PR113580]
2024-01-26 15:49 ` Patrick Palka
@ 2024-01-26 19:54 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2024-01-26 19:54 UTC (permalink / raw)
To: Patrick Palka, Nathaniel Shead; +Cc: gcc-patches, Nathan Sidwell
On 1/26/24 10:49, Patrick Palka wrote:
> 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
Yep, OK.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-26 19:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-26 11:28 [PATCH] c++/modules: Stream additional fields for DECL_STRUCT_FUNCTION [PR113580] Nathaniel Shead
2024-01-26 15:49 ` Patrick Palka
2024-01-26 19:54 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).