https://gcc.gnu.org/g:a96c774f7bb99729ab9e7e2a57cd970469ccbc08
commit r15-4944-ga96c774f7bb99729ab9e7e2a57cd970469ccbc08 Author: Nathaniel Shead <nathanielosh...@gmail.com> Date: Mon Aug 12 10:57:39 2024 +1000 c++/modules: Merge default arguments [PR99274] 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. * tree.cc (cp_tree_equal) <AGGR_INIT_EXPR>: Handle unification of AGGR_INIT_EXPRs with new VAR_DECL slots. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7.h: Skip ODR-violating declaration when testing ODR deduplication. * g++.dg/modules/lambda-7_b.C: Note we're testing ODR deduplication. * 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> Reviewed-by: Patrick Palka <ppa...@redhat.com> Reviewed-by: Jason Merrill <ja...@redhat.com> Diff: --- gcc/cp/module.cc | 57 ++++++++++++++++++++++++-- gcc/cp/tree.cc | 33 +++++++++++++++ gcc/testsuite/g++.dg/modules/default-arg-1_a.H | 17 ++++++++ gcc/testsuite/g++.dg/modules/default-arg-1_b.C | 26 ++++++++++++ gcc/testsuite/g++.dg/modules/default-arg-2_a.H | 17 ++++++++ gcc/testsuite/g++.dg/modules/default-arg-2_b.C | 28 +++++++++++++ gcc/testsuite/g++.dg/modules/default-arg-3.h | 13 ++++++ gcc/testsuite/g++.dg/modules/default-arg-3_a.H | 5 +++ gcc/testsuite/g++.dg/modules/default-arg-3_b.C | 6 +++ gcc/testsuite/g++.dg/modules/lambda-7.h | 6 +++ gcc/testsuite/g++.dg/modules/lambda-7_b.C | 1 + 11 files changed, 206 insertions(+), 3 deletions(-) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index e64e6a2103ae..4eefb2d35840 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -11585,8 +11585,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 @@ -11725,7 +11723,60 @@ 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; + } + } + } + + 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)) + { + auto_diagnostic_group d; + error_at (get_fndecl_argument_location (d_inner, i), + "conflicting default argument for parameter %P of %#qD", + i, decl); + inform (get_fndecl_argument_location (e_inner, i), + "existing default declared here"); + return false; + } + } + } return true; } diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index 911260685f1a..8e566cadcaf2 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -4042,6 +4042,39 @@ cp_tree_equal (tree t1, tree t2) TARGET_EXPR_INITIAL (t2)); } + case AGGR_INIT_EXPR: + { + int n = aggr_init_expr_nargs (t1); + if (n != aggr_init_expr_nargs (t2)) + return false; + + if (!cp_tree_equal (AGGR_INIT_EXPR_FN (t1), + AGGR_INIT_EXPR_FN (t2))) + return false; + + tree o1 = AGGR_INIT_EXPR_SLOT (t1); + tree o2 = AGGR_INIT_EXPR_SLOT (t2); + + /* Similarly to TARGET_EXPRs, if the VAR_DECL is unallocated we're + going to unify the initialization, so treat it as equivalent + to anything. */ + if (VAR_P (o1) && DECL_NAME (o1) == NULL_TREE + && !DECL_RTL_SET_P (o1)) + /*Nop*/; + else if (VAR_P (o2) && DECL_NAME (o2) == NULL_TREE + && !DECL_RTL_SET_P (o2)) + /*Nop*/; + else if (!cp_tree_equal (o1, o2)) + return false; + + for (int i = 0; i < n; ++i) + if (!cp_tree_equal (AGGR_INIT_EXPR_ARG (t1, i), + AGGR_INIT_EXPR_ARG (t2, i))) + return false; + + return true; + } + case PARM_DECL: /* For comparing uses of parameters in late-specified return types with an out-of-class definition of the function, but can also come diff --git a/gcc/testsuite/g++.dg/modules/default-arg-1_a.H b/gcc/testsuite/g++.dg/modules/default-arg-1_a.H new file mode 100644 index 000000000000..65334930f744 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-1_a.H @@ -0,0 +1,17 @@ +// PR c++/99274 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +void f(int a, int b = 123); +template <typename T> void g(T a, T b = {}); + +template <typename T, typename U = int> struct A; +template <typename T, int N = 5> struct B; + +struct S { + template <typename T = int> void x(); + void y(int n = 123); +}; + +struct nontrivial { nontrivial(int); }; +void h(nontrivial p = nontrivial(123)); diff --git a/gcc/testsuite/g++.dg/modules/default-arg-1_b.C b/gcc/testsuite/g++.dg/modules/default-arg-1_b.C new file mode 100644 index 000000000000..80822896de1a --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-1_b.C @@ -0,0 +1,26 @@ +// PR c++/99274 +// { dg-additional-options "-fmodules-ts" } +// Propagate default args from import to existing decls + +void f(int a, int b); +template <typename T> void g(T a, T b); +template <typename T, typename U> struct A; +template <typename T, int N> struct B; +struct S { + template <typename T = int> void x(); + void y(int n = 123); +}; +struct nontrivial { nontrivial(int); }; +void h(nontrivial p); + +import "default-arg-1_a.H"; + +template <typename = A<int>, typename = B<int>> struct Q; + +int main() { + f(1); + g(2); + h(); + S{}.x(); + S{}.y(); +} diff --git a/gcc/testsuite/g++.dg/modules/default-arg-2_a.H b/gcc/testsuite/g++.dg/modules/default-arg-2_a.H new file mode 100644 index 000000000000..085d4c7f7929 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-2_a.H @@ -0,0 +1,17 @@ +// PR c++/99274 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +void f(int a, int b = 123); +template <typename T> void g(T a, T b = 123); + +template <typename U = int> struct A; +template <int N = 123> struct B; + +struct S { + template <typename T = int> void x(); + void y(int n = 123); +}; + +struct nontrivial { nontrivial(int); }; +void h(nontrivial p = nontrivial(123)); diff --git a/gcc/testsuite/g++.dg/modules/default-arg-2_b.C b/gcc/testsuite/g++.dg/modules/default-arg-2_b.C new file mode 100644 index 000000000000..b1985c08778d --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-2_b.C @@ -0,0 +1,28 @@ +// PR c++/99274 +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } +// Check for conflicting defaults. + +void f(int a, int b = 456); // { dg-message "existing default declared here" } + +template <typename T> +void g(T a, T b = {}); // { dg-message "existing default declared here" } + +template <typename U = double> // { dg-message "existing default declared here" } +struct A; + +template <int N = 456> // { dg-message "existing default declared here" } +struct B; + +struct S { + template <typename T = double> // { dg-message "existing default declared here" } + void x(); + + void y(int n = 456); // { dg-message "existing default declared here" } +}; + +struct nontrivial { nontrivial(int); }; +void h(nontrivial p = nontrivial(456)); // { dg-message "existing default declared here" } + +import "default-arg-2_a.H"; + +// { dg-error "conflicting default argument" "" { target *-*-* } 0 } diff --git a/gcc/testsuite/g++.dg/modules/default-arg-3.h b/gcc/testsuite/g++.dg/modules/default-arg-3.h new file mode 100644 index 000000000000..b55a6690f4ab --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-3.h @@ -0,0 +1,13 @@ +void f(int a, int b = 123); +template <typename T> void g(T a, T b = 123); + +template <typename U = int> struct A; +template <int N = 123> struct B; + +struct S { + template <typename T = int> void x(); + void y(int n = 123); +}; + +struct nontrivial { nontrivial(int); }; +void h(nontrivial p = nontrivial(123)); diff --git a/gcc/testsuite/g++.dg/modules/default-arg-3_a.H b/gcc/testsuite/g++.dg/modules/default-arg-3_a.H new file mode 100644 index 000000000000..879bedce67fc --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-3_a.H @@ -0,0 +1,5 @@ +// PR c++/99274 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +#include "default-arg-3.h" diff --git a/gcc/testsuite/g++.dg/modules/default-arg-3_b.C b/gcc/testsuite/g++.dg/modules/default-arg-3_b.C new file mode 100644 index 000000000000..402dca6ff76d --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/default-arg-3_b.C @@ -0,0 +1,6 @@ +// PR c++/99274 +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } +// Don't complain about matching defaults. + +#include "default-arg-3.h" +import "default-arg-3_a.H"; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h index 6f6080c13240..bfae142ed91d 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-7.h +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h @@ -10,9 +10,15 @@ struct S { } }; +// this is an ODR violation to include this header in multiple TUs +// (the lambda is a different type in each TU, see [basic.def.odr] p15.6); +// keep the check that we can emit the lambda from a header unit, +// but skip it when checking ODR deduplication. +#ifndef CHECK_ODR_VIOLATIONS inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { return f(x); } +#endif // unevaluated lambdas #if __cplusplus >= 202002L diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C index 2d781e930678..50a3a568d1c8 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-7_b.C +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C @@ -1,5 +1,6 @@ // { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } // Test for ODR deduplication +#define CHECK_ODR_VIOLATIONS #include "lambda-7.h" import "lambda-7_a.H";