On Mon, 12 Aug 2024, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
I tried to implement a remapping of the slots for TARGET_EXPRs for the
FIXME but I wasn't able to work out how to do so effectively. Given
that I doubt this will be a common issue I felt probably easiest to
leave it for now and focus on other issues in the meantime; thoughts?
The other thing to note is that most of this function just has a single
error message always indicated by a 'goto mismatch;' but I felt that it
seemed reasonable to provide more specific error messages where we can.
But given that in the long term we probably want to replace this
function with an appropriately enhanced 'duplicate_decls' anyway maybe
it's not worth worrying about; this patch is still useful in the
meantime if only for the testcases, I hope.
-- >8 --
When merging a newly imported declaration with an existing declaration
we don't currently propagate new default arguments, which causes issues
when modularising header units. This patch adds logic to propagate
default arguments to existing declarations on import, and error if the
defaults do not match.
PR c++/99274
gcc/cp/ChangeLog:
* module.cc (trees_in::is_matching_decl): Merge default
arguments.
gcc/testsuite/ChangeLog:
* g++.dg/modules/default-arg-1_a.H: New test.
* g++.dg/modules/default-arg-1_b.C: New test.
* g++.dg/modules/default-arg-2_a.H: New test.
* g++.dg/modules/default-arg-2_b.C: New test.
* g++.dg/modules/default-arg-3.h: New test.
* g++.dg/modules/default-arg-3_a.H: New test.
* g++.dg/modules/default-arg-3_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 62 ++++++++++++++++++-
.../g++.dg/modules/default-arg-1_a.H | 17 +++++
.../g++.dg/modules/default-arg-1_b.C | 26 ++++++++
.../g++.dg/modules/default-arg-2_a.H | 17 +++++
.../g++.dg/modules/default-arg-2_b.C | 28 +++++++++
gcc/testsuite/g++.dg/modules/default-arg-3.h | 13 ++++
.../g++.dg/modules/default-arg-3_a.H | 5 ++
.../g++.dg/modules/default-arg-3_b.C | 6 ++
8 files changed, 171 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-1_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-1_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-2_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-2_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-3.h
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-3_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/default-arg-3_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f4d137b13a1..87f34bac578 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11551,8 +11551,6 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
if (!same_type_p (TREE_VALUE (d_args), TREE_VALUE (e_args)))
goto mismatch;
-
- // FIXME: Check default values
}
/* If EXISTING has an undeduced or uninstantiated exception
@@ -11690,7 +11688,65 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
if (!DECL_EXTERNAL (d_inner))
DECL_EXTERNAL (e_inner) = false;
- // FIXME: Check default tmpl and fn parms here
+ if (TREE_CODE (decl) == TEMPLATE_DECL)
+ {
+ /* Merge default template arguments. */
+ tree d_parms = DECL_INNERMOST_TEMPLATE_PARMS (decl);
+ tree e_parms = DECL_INNERMOST_TEMPLATE_PARMS (existing);
+ gcc_checking_assert (TREE_VEC_LENGTH (d_parms)
+ == TREE_VEC_LENGTH (e_parms));
+ for (int i = 0; i < TREE_VEC_LENGTH (d_parms); ++i)
+ {
+ tree d_default = TREE_PURPOSE (TREE_VEC_ELT (d_parms, i));
+ tree& e_default = TREE_PURPOSE (TREE_VEC_ELT (e_parms, i));
+ if (e_default == NULL_TREE)
+ e_default = d_default;
+ else if (d_default != NULL_TREE
+ && !cp_tree_equal (d_default, e_default))
+ {
+ auto_diagnostic_group d;
+ tree d_parm = TREE_VALUE (TREE_VEC_ELT (d_parms, i));
+ tree e_parm = TREE_VALUE (TREE_VEC_ELT (e_parms, i));
+ error_at (DECL_SOURCE_LOCATION (d_parm),
+ "conflicting default argument for %#qD", d_parm);
+ inform (DECL_SOURCE_LOCATION (e_parm),
+ "existing default declared here");
+ return false;
+ }
+ }
+ }
FWIW for ordinary textual redeclarations we merge default template
arguments via merge_default_template_args. But the semantics are
slightly different on stream in because we want to allow a default
argument redefinition as long as it's cp_tree_equal, so I don't think
we can or want to use merge_default_template_args here.
+
+ if (TREE_CODE (d_inner) == FUNCTION_DECL)
+ {
+ /* Merge default function arguments. */
+ tree d_parm = FUNCTION_FIRST_USER_PARMTYPE (d_inner);
+ tree e_parm = FUNCTION_FIRST_USER_PARMTYPE (e_inner);
+ int i = 0;
+ for (; d_parm && d_parm != void_list_node;
+ d_parm = TREE_CHAIN (d_parm), e_parm = TREE_CHAIN (e_parm), ++i)
+ {
+ tree d_default = TREE_PURPOSE (d_parm);
+ tree& e_default = TREE_PURPOSE (e_parm);
+ if (e_default == NULL_TREE)
+ e_default = d_default;
+ else if (d_default != NULL_TREE
+ && !cp_tree_equal (d_default, e_default)
+ /* FIXME: We need to remap the target to match the existing
+ parms so that cp_tree_equal works correctly, as with
+ break_out_target_exprs. */
Is the problem that cp_tree_equal expects TARGET_EXPR_SLOT to be
identical (despite the probably bitrotted special case for empty
DECL_NAME)? Maybe it'd make sense to always only compare
TARGET_EXPR_INITIAL, not TARGET_EXPR_SLOT..
