* [PATCH] c++/modules: anon union member of as-base class [PR112580]
@ 2024-02-01 17:18 Patrick Palka
2024-02-08 13:58 ` Patrick Palka
2024-02-09 16:49 ` Jason Merrill
0 siblings, 2 replies; 3+ messages in thread
From: Patrick Palka @ 2024-02-01 17:18 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
-- >8 --
Here when streaming in the fields of the as-base version of
_Formatting_scanner<int> we end up clobbering ANON_AGGR_TYPE_FIELD
of the anonymous union type, since it turns out this type is shared
between the original FIELD_DECL and the as-base FIELD_DECL copy (copied
during layout_class_type). ANON_AGGR_TYPE_FIELD first gets properly set
to the original FIELD_DECL when streaming in the canonical definition of
_Formatting_scanner<int>, and then gets overwritten to the as-base
FIELD_DECL when streaming in the the as-base definition. This leads to
lookup_anon_field giving the wrong answer when resolving the _M_values
use.
This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming
in an as-base class definition, it should already be properly set at that
point.
PR c++/112580
gcc/cp/ChangeLog:
* module.cc (trees_in::read_class_def): When streaming in
an anonymous union field of an as-base class, don't clobber
ANON_AGGR_TYPE_FIELD.
gcc/testsuite/ChangeLog:
* g++.dg/modules/anon-3_a.H: New test.
* g++.dg/modules/anon-3_b.C: New test.
---
gcc/cp/module.cc | 12 ++++++++++--
gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +++++++++++++++++++++
gcc/testsuite/g++.dg/modules/anon-3_b.C | 8 ++++++++
3 files changed, 39 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3c2fef0e3f4..d36ba2eeeb3 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template)
if (TREE_CODE (decl) == FIELD_DECL
&& ANON_AGGR_TYPE_P (TREE_TYPE (decl)))
- ANON_AGGR_TYPE_FIELD
- (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl;
+ {
+ tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
+ if (DECL_NAME (defn) == as_base_identifier)
+ /* ANON_AGGR_TYPE_FIELD should already point to the
+ original FIELD_DECL, don't overwrite it to point
+ to the as-base FIELD_DECL copy. */
+ gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type));
+ else
+ ANON_AGGR_TYPE_FIELD (anon_type) = decl;
+ }
if (TREE_CODE (decl) == USING_DECL
&& TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE)
diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H
new file mode 100644
index 00000000000..64a6aab51dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H
@@ -0,0 +1,21 @@
+// PR c++/112580
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename _Out>
+struct _Formatting_scanner {
+ union {
+ int _M_values = 42;
+ };
+ virtual int _M_format_arg() { return _M_values; }
+};
+
+template <typename _Tp>
+auto __do_vformat_to() {
+ _Formatting_scanner<int> s;
+ return s;
+}
+
+inline auto vformat() {
+ return __do_vformat_to<int>();
+}
diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C
new file mode 100644
index 00000000000..d676fe47536
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C
@@ -0,0 +1,8 @@
+// PR c++/112580
+// { dg-additional-options "-fmodules-ts" }
+
+import "anon-3_a.H";
+
+int main() {
+ vformat()._M_format_arg();
+}
--
2.43.0.493.gbc7ee2e5e1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++/modules: anon union member of as-base class [PR112580]
2024-02-01 17:18 [PATCH] c++/modules: anon union member of as-base class [PR112580] Patrick Palka
@ 2024-02-08 13:58 ` Patrick Palka
2024-02-09 16:49 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Patrick Palka @ 2024-02-08 13:58 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, jason
On Thu, 1 Feb 2024, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
Ping. FWIW this fixes the remaining reported xtreme-header FAILs on
x86_64-linux with -mx32 (not totally sure why the fails don't happen
without -mx32).
>
> -- >8 --
>
> Here when streaming in the fields of the as-base version of
> _Formatting_scanner<int> we end up clobbering ANON_AGGR_TYPE_FIELD
> of the anonymous union type, since it turns out this type is shared
> between the original FIELD_DECL and the as-base FIELD_DECL copy (copied
> during layout_class_type). ANON_AGGR_TYPE_FIELD first gets properly set
> to the original FIELD_DECL when streaming in the canonical definition of
> _Formatting_scanner<int>, and then gets overwritten to the as-base
> FIELD_DECL when streaming in the the as-base definition. This leads to
> lookup_anon_field giving the wrong answer when resolving the _M_values
> use.
>
> This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming
> in an as-base class definition, it should already be properly set at that
> point.
>
> PR c++/112580
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_in::read_class_def): When streaming in
> an anonymous union field of an as-base class, don't clobber
> ANON_AGGR_TYPE_FIELD.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/anon-3_a.H: New test.
> * g++.dg/modules/anon-3_b.C: New test.
> ---
> gcc/cp/module.cc | 12 ++++++++++--
> gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +++++++++++++++++++++
> gcc/testsuite/g++.dg/modules/anon-3_b.C | 8 ++++++++
> 3 files changed, 39 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 3c2fef0e3f4..d36ba2eeeb3 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template)
>
> if (TREE_CODE (decl) == FIELD_DECL
> && ANON_AGGR_TYPE_P (TREE_TYPE (decl)))
> - ANON_AGGR_TYPE_FIELD
> - (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl;
> + {
> + tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
> + if (DECL_NAME (defn) == as_base_identifier)
> + /* ANON_AGGR_TYPE_FIELD should already point to the
> + original FIELD_DECL, don't overwrite it to point
> + to the as-base FIELD_DECL copy. */
> + gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type));
> + else
> + ANON_AGGR_TYPE_FIELD (anon_type) = decl;
> + }
>
> if (TREE_CODE (decl) == USING_DECL
> && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE)
> diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H
> new file mode 100644
> index 00000000000..64a6aab51dd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H
> @@ -0,0 +1,21 @@
> +// PR c++/112580
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +template <typename _Out>
> +struct _Formatting_scanner {
> + union {
> + int _M_values = 42;
> + };
> + virtual int _M_format_arg() { return _M_values; }
> +};
> +
> +template <typename _Tp>
> +auto __do_vformat_to() {
> + _Formatting_scanner<int> s;
> + return s;
> +}
> +
> +inline auto vformat() {
> + return __do_vformat_to<int>();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C
> new file mode 100644
> index 00000000000..d676fe47536
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C
> @@ -0,0 +1,8 @@
> +// PR c++/112580
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "anon-3_a.H";
> +
> +int main() {
> + vformat()._M_format_arg();
> +}
> --
> 2.43.0.493.gbc7ee2e5e1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++/modules: anon union member of as-base class [PR112580]
2024-02-01 17:18 [PATCH] c++/modules: anon union member of as-base class [PR112580] Patrick Palka
2024-02-08 13:58 ` Patrick Palka
@ 2024-02-09 16:49 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2024-02-09 16:49 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
On 2/1/24 12:18, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
OK.
> -- >8 --
>
> Here when streaming in the fields of the as-base version of
> _Formatting_scanner<int> we end up clobbering ANON_AGGR_TYPE_FIELD
> of the anonymous union type, since it turns out this type is shared
> between the original FIELD_DECL and the as-base FIELD_DECL copy (copied
> during layout_class_type). ANON_AGGR_TYPE_FIELD first gets properly set
> to the original FIELD_DECL when streaming in the canonical definition of
> _Formatting_scanner<int>, and then gets overwritten to the as-base
> FIELD_DECL when streaming in the the as-base definition. This leads to
> lookup_anon_field giving the wrong answer when resolving the _M_values
> use.
>
> This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming
> in an as-base class definition, it should already be properly set at that
> point.
>
> PR c++/112580
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_in::read_class_def): When streaming in
> an anonymous union field of an as-base class, don't clobber
> ANON_AGGR_TYPE_FIELD.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/anon-3_a.H: New test.
> * g++.dg/modules/anon-3_b.C: New test.
> ---
> gcc/cp/module.cc | 12 ++++++++++--
> gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +++++++++++++++++++++
> gcc/testsuite/g++.dg/modules/anon-3_b.C | 8 ++++++++
> 3 files changed, 39 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 3c2fef0e3f4..d36ba2eeeb3 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template)
>
> if (TREE_CODE (decl) == FIELD_DECL
> && ANON_AGGR_TYPE_P (TREE_TYPE (decl)))
> - ANON_AGGR_TYPE_FIELD
> - (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl;
> + {
> + tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
> + if (DECL_NAME (defn) == as_base_identifier)
> + /* ANON_AGGR_TYPE_FIELD should already point to the
> + original FIELD_DECL, don't overwrite it to point
> + to the as-base FIELD_DECL copy. */
> + gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type));
> + else
> + ANON_AGGR_TYPE_FIELD (anon_type) = decl;
> + }
>
> if (TREE_CODE (decl) == USING_DECL
> && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE)
> diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H
> new file mode 100644
> index 00000000000..64a6aab51dd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H
> @@ -0,0 +1,21 @@
> +// PR c++/112580
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +template <typename _Out>
> +struct _Formatting_scanner {
> + union {
> + int _M_values = 42;
> + };
> + virtual int _M_format_arg() { return _M_values; }
> +};
> +
> +template <typename _Tp>
> +auto __do_vformat_to() {
> + _Formatting_scanner<int> s;
> + return s;
> +}
> +
> +inline auto vformat() {
> + return __do_vformat_to<int>();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C
> new file mode 100644
> index 00000000000..d676fe47536
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C
> @@ -0,0 +1,8 @@
> +// PR c++/112580
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "anon-3_a.H";
> +
> +int main() {
> + vformat()._M_format_arg();
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-09 16:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 17:18 [PATCH] c++/modules: anon union member of as-base class [PR112580] Patrick Palka
2024-02-08 13:58 ` Patrick Palka
2024-02-09 16:49 ` 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).