| Message ID | 20201112182723.2729551-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Don't form a templated TARGET_EXPR in finish_compound_literal | expand |
On Thu, Nov 12, 2020 at 01:27:23PM -0500, Patrick Palka wrote: > The atom_cache in normalize_atom relies on the assumption that two > equivalent (templated) trees (in the sense of cp_tree_equal) must use > the same template parameters (according to find_template_parameters). > > This assumption unfortunately doesn't always hold for TARGET_EXPRs, > because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but > find_template_parameters walks this target (and its DECL_CONTEXT). > > Hence two TARGET_EXPRs built by force_target_expr with the same > initializer but under different settings of current_function_decl may > compare equal according to cp_tree_equal, but find_template_parameters > returns a different set of template parameters for them. This breaks > the below testcase because during normalization we build two such > TARGET_EXPRs (one under current_function_decl=f and another under =g), > and then use the same ATOMIC_CONSTR for the two corresponding atoms, > leading to a crash during satisfaction of g's associated constraints. > > This patch works around this assumption violation by removing the source > of these templated TARGET_EXPRs. The relevant call to get_target_expr was > added in r9-6043, but it seems it's no longer necessary (according to > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the > call was added in order to avoid regressing on initlist109.C at the time). > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? Looks OK to me, thanks! > gcc/cp/ChangeLog: > > * semantics.c (finish_compound_literal): Don't wrap the original > compound literal in a TARGET_EXPR when inside a template. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-decltype3.C: New test. > --- > gcc/cp/semantics.c | 7 +------ > gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C | 15 +++++++++++++++ > 2 files changed, 16 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > index 33d715edaec..172286922e7 100644 > --- a/gcc/cp/semantics.c > +++ b/gcc/cp/semantics.c > @@ -3006,12 +3006,7 @@ finish_compound_literal (tree type, tree compound_literal, > > /* If we're in a template, return the original compound literal. */ > if (orig_cl) > - { > - if (!VECTOR_TYPE_P (type)) > - return get_target_expr_sfinae (orig_cl, complain); > - else > - return orig_cl; > - } > + return orig_cl; > > if (TREE_CODE (compound_literal) == CONSTRUCTOR) > { > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > new file mode 100644 > index 00000000000..837855ce8ac > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > @@ -0,0 +1,15 @@ > +// { dg-do compile { target c++20 } } > + > +template <class T> concept C = requires(T t) { t; }; > + > +template <class T> using A = decltype((T{}, int{})); > + > +template <class T> concept D = C<A<T>>; > + > +template <class T, class U> void f() requires D<T>; > +template <class T> void g() requires D<T>; > + > +void h() { > + f<int, int>(); > + g<int>(); > +} > -- > 2.29.2.260.ge31aba42fb > Marek
On 11/12/20 1:27 PM, Patrick Palka wrote: > The atom_cache in normalize_atom relies on the assumption that two > equivalent (templated) trees (in the sense of cp_tree_equal) must use > the same template parameters (according to find_template_parameters). > > This assumption unfortunately doesn't always hold for TARGET_EXPRs, > because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but > find_template_parameters walks this target (and its DECL_CONTEXT). > > Hence two TARGET_EXPRs built by force_target_expr with the same > initializer but under different settings of current_function_decl may > compare equal according to cp_tree_equal, but find_template_parameters > returns a different set of template parameters for them. This breaks > the below testcase because during normalization we build two such > TARGET_EXPRs (one under current_function_decl=f and another under =g), > and then use the same ATOMIC_CONSTR for the two corresponding atoms, > leading to a crash during satisfaction of g's associated constraints. > > This patch works around this assumption violation by removing the source > of these templated TARGET_EXPRs. The relevant call to get_target_expr was > added in r9-6043, but it seems it's no longer necessary (according to > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the > call was added in order to avoid regressing on initlist109.C at the time). > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? OK. I wonder what else asserting !processing_template_decl in build_target_expr would find... > gcc/cp/ChangeLog: > > * semantics.c (finish_compound_literal): Don't wrap the original > compound literal in a TARGET_EXPR when inside a template. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-decltype3.C: New test. > --- > gcc/cp/semantics.c | 7 +------ > gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C | 15 +++++++++++++++ > 2 files changed, 16 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > index 33d715edaec..172286922e7 100644 > --- a/gcc/cp/semantics.c > +++ b/gcc/cp/semantics.c > @@ -3006,12 +3006,7 @@ finish_compound_literal (tree type, tree compound_literal, > > /* If we're in a template, return the original compound literal. */ > if (orig_cl) > - { > - if (!VECTOR_TYPE_P (type)) > - return get_target_expr_sfinae (orig_cl, complain); > - else > - return orig_cl; > - } > + return orig_cl; > > if (TREE_CODE (compound_literal) == CONSTRUCTOR) > { > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > new file mode 100644 > index 00000000000..837855ce8ac > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > @@ -0,0 +1,15 @@ > +// { dg-do compile { target c++20 } } > + > +template <class T> concept C = requires(T t) { t; }; > + > +template <class T> using A = decltype((T{}, int{})); > + > +template <class T> concept D = C<A<T>>; > + > +template <class T, class U> void f() requires D<T>; > +template <class T> void g() requires D<T>; > + > +void h() { > + f<int, int>(); > + g<int>(); > +} >
On Thu, 12 Nov 2020, Jason Merrill wrote: > On 11/12/20 1:27 PM, Patrick Palka wrote: > > The atom_cache in normalize_atom relies on the assumption that two > > equivalent (templated) trees (in the sense of cp_tree_equal) must use > > the same template parameters (according to find_template_parameters). > > > > This assumption unfortunately doesn't always hold for TARGET_EXPRs, > > because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but > > find_template_parameters walks this target (and its DECL_CONTEXT). > > > > Hence two TARGET_EXPRs built by force_target_expr with the same > > initializer but under different settings of current_function_decl may > > compare equal according to cp_tree_equal, but find_template_parameters > > returns a different set of template parameters for them. This breaks > > the below testcase because during normalization we build two such > > TARGET_EXPRs (one under current_function_decl=f and another under =g), > > and then use the same ATOMIC_CONSTR for the two corresponding atoms, > > leading to a crash during satisfaction of g's associated constraints. > > > > This patch works around this assumption violation by removing the source > > of these templated TARGET_EXPRs. The relevant call to get_target_expr was > > added in r9-6043, but it seems it's no longer necessary (according to > > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the > > call was added in order to avoid regressing on initlist109.C at the time). > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > OK. I wonder what else asserting !processing_template_decl in > build_target_expr would find... FWIW, testing exposed seven distinct paths that trigger such an assert, five of which go through build_cplus_new: 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) /gcc/gcc/cp/tree.c:728 0x91712b force_rvalue(tree_node*, int) /gcc/gcc/cp/cvt.c:569 0x8b8310 build_conditional_expr_1 /gcc/gcc/cp/call.c:5592 0x8ba08c build_conditional_expr(op_location_t const&, tree_node*, tree_node*, tree_node*, int) /gcc/gcc/cp/call.c:5777 0xaa70fb build_x_conditional_expr(unsigned int, tree_node*, tree_node*, tree_node*, int) /gcc/gcc/cp/typeck.c:7133 0x9cc9fa cp_parser_assignment_expression /gcc/gcc/cp/parser.c:9964 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) /gcc/gcc/cp/tree.c:728 0x97d185 expand_default_init /gcc/gcc/cp/init.c:1924 0x97d185 expand_aggr_init_1 /gcc/gcc/cp/init.c:2101 0x97f026 build_aggr_init(tree_node*, tree_node*, int, int) /gcc/gcc/cp/init.c:1835 0x92c88d build_aggr_init_full_exprs /gcc/gcc/cp/decl.c:6696 0x92c88d check_initializer /gcc/gcc/cp/decl.c:6857 0x950982 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) /gcc/gcc/cp/decl.c:7699 0x960c7e grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, bool, tree_node*, tree_node*) /gcc/gcc/cp/decl2.c:1000 0xa02ceb cp_parser_member_declaration /gcc/gcc/cp/parser.c:25755 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) /gcc/gcc/cp/tree.c:728 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) /gcc/gcc/cp/call.c:9747 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, tree_node*) /gcc/gcc/cp/typeck.c:4025 0xa75a30 finish_call_expr(tree_node*, vec<tree_node*, va_gc, vl_embed>**, bool, bool, int) /gcc/gcc/cp/semantics.c:2728 0x9e9383 cp_parser_postfix_expression /gcc/gcc/cp/parser.c:7549 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) /gcc/gcc/cp/tree.c:728 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) /gcc/gcc/cp/call.c:9747 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, tree_node*) /gcc/gcc/cp/typeck.c:4025 0x95eb40 build_offset_ref_call_from_tree(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int) /gcc/gcc/cp/decl2.c:5292 0x9e9b1f cp_parser_postfix_expression /gcc/gcc/cp/parser.c:7534 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) /gcc/gcc/cp/tree.c:728 0x8b67c2 perform_direct_initialization_if_possible(tree_node*, tree_node*, bool, int) /gcc/gcc/cp/call.c:12038 0xaba6a9 build_static_cast_1 /gcc/gcc/cp/typeck.c:7607 0xabb500 build_static_cast(unsigned int, tree_node*, tree_node*, int) /gcc/gcc/cp/typeck.c:7813 0x9e9f9e cp_parser_postfix_expression /gcc/gcc/cp/parser.c:7049 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0x97fb53 build_new_1 /gcc/gcc/cp/init.c:3281 0x982382 build_new(unsigned int, vec<tree_node*, va_gc, vl_embed>**, tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, int) /gcc/gcc/cp/init.c:3817 0x9f2107 cp_parser_new_expression /gcc/gcc/cp/parser.c:8919 0x6b3983 build_target_expr /gcc/gcc/cp/tree.c:496 0xaa2b0f get_target_expr(tree_node*) /gcc/gcc/cp/tree.c:899 0xaa2b0f stabilize_expr(tree_node*, tree_node**) /gcc/gcc/cp/tree.c:5509 0xabfa2a cp_build_modify_expr(unsigned int, tree_node*, tree_code, tree_node*, int) /gcc/gcc/cp/typeck.c:8736 0x8b7c91 build_new_op_1 /gcc/gcc/cp/call.c:6537 0x8b809d build_new_op(op_location_t const&, tree_code, int, tree_node*, tree_node*, tree_node*, tree_node**, int) /gcc/gcc/cp/call.c:6623 0xac0b04 build_x_modify_expr(unsigned int, tree_node*, tree_code, tree_node*, int) /gcc/gcc/cp/typeck.c:8936 0x9ccafe cp_parser_assignment_expression /gcc/gcc/cp/parser.c:10002 > > > gcc/cp/ChangeLog: > > > > * semantics.c (finish_compound_literal): Don't wrap the original > > compound literal in a TARGET_EXPR when inside a template. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp2a/concepts-decltype3.C: New test. > > --- > > gcc/cp/semantics.c | 7 +------ > > gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C | 15 +++++++++++++++ > > 2 files changed, 16 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > > > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c > > index 33d715edaec..172286922e7 100644 > > --- a/gcc/cp/semantics.c > > +++ b/gcc/cp/semantics.c > > @@ -3006,12 +3006,7 @@ finish_compound_literal (tree type, tree > > compound_literal, > > /* If we're in a template, return the original compound literal. */ > > if (orig_cl) > > - { > > - if (!VECTOR_TYPE_P (type)) > > - return get_target_expr_sfinae (orig_cl, complain); > > - else > > - return orig_cl; > > - } > > + return orig_cl; > > if (TREE_CODE (compound_literal) == CONSTRUCTOR) > > { > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > new file mode 100644 > > index 00000000000..837855ce8ac > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C > > @@ -0,0 +1,15 @@ > > +// { dg-do compile { target c++20 } } > > + > > +template <class T> concept C = requires(T t) { t; }; > > + > > +template <class T> using A = decltype((T{}, int{})); > > + > > +template <class T> concept D = C<A<T>>; > > + > > +template <class T, class U> void f() requires D<T>; > > +template <class T> void g() requires D<T>; > > + > > +void h() { > > + f<int, int>(); > > + g<int>(); > > +} > > > >
On 11/13/20 10:43 AM, Patrick Palka wrote: > On Thu, 12 Nov 2020, Jason Merrill wrote: > >> On 11/12/20 1:27 PM, Patrick Palka wrote: >>> The atom_cache in normalize_atom relies on the assumption that two >>> equivalent (templated) trees (in the sense of cp_tree_equal) must use >>> the same template parameters (according to find_template_parameters). >>> >>> This assumption unfortunately doesn't always hold for TARGET_EXPRs, >>> because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but >>> find_template_parameters walks this target (and its DECL_CONTEXT). >>> >>> Hence two TARGET_EXPRs built by force_target_expr with the same >>> initializer but under different settings of current_function_decl may >>> compare equal according to cp_tree_equal, but find_template_parameters >>> returns a different set of template parameters for them. This breaks >>> the below testcase because during normalization we build two such >>> TARGET_EXPRs (one under current_function_decl=f and another under =g), >>> and then use the same ATOMIC_CONSTR for the two corresponding atoms, >>> leading to a crash during satisfaction of g's associated constraints. >>> >>> This patch works around this assumption violation by removing the source >>> of these templated TARGET_EXPRs. The relevant call to get_target_expr was >>> added in r9-6043, but it seems it's no longer necessary (according to >>> https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the >>> call was added in order to avoid regressing on initlist109.C at the time). >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>> trunk? >> >> OK. I wonder what else asserting !processing_template_decl in >> build_target_expr would find... > > FWIW, testing exposed seven distinct paths that trigger such an assert, > five of which go through build_cplus_new: Most of these are built as part of overload resolution and then thrown away, only using the result for its type. But I wonder about the build_aggr_init instance. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > /gcc/gcc/cp/tree.c:728 > 0x91712b force_rvalue(tree_node*, int) > /gcc/gcc/cp/cvt.c:569 > 0x8b8310 build_conditional_expr_1 > /gcc/gcc/cp/call.c:5592 > 0x8ba08c build_conditional_expr(op_location_t const&, tree_node*, tree_node*, tree_node*, int) > /gcc/gcc/cp/call.c:5777 > 0xaa70fb build_x_conditional_expr(unsigned int, tree_node*, tree_node*, tree_node*, int) > /gcc/gcc/cp/typeck.c:7133 > 0x9cc9fa cp_parser_assignment_expression > /gcc/gcc/cp/parser.c:9964 This one gets discarded by build_x_conditional_expr. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > /gcc/gcc/cp/tree.c:728 > 0x97d185 expand_default_init > /gcc/gcc/cp/init.c:1924 > 0x97d185 expand_aggr_init_1 > /gcc/gcc/cp/init.c:2101 > 0x97f026 build_aggr_init(tree_node*, tree_node*, int, int) > /gcc/gcc/cp/init.c:1835 > 0x92c88d build_aggr_init_full_exprs > /gcc/gcc/cp/decl.c:6696 > 0x92c88d check_initializer > /gcc/gcc/cp/decl.c:6857 > 0x950982 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) > /gcc/gcc/cp/decl.c:7699 > 0x960c7e grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, bool, tree_node*, tree_node*) > /gcc/gcc/cp/decl2.c:1000 > 0xa02ceb cp_parser_member_declaration > /gcc/gcc/cp/parser.c:25755 This one looks dubious. Which testcase? > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) > /gcc/gcc/cp/tree.c:728 > 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) > /gcc/gcc/cp/call.c:9747 > 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, tree_node*) > /gcc/gcc/cp/typeck.c:4025 > 0xa75a30 finish_call_expr(tree_node*, vec<tree_node*, va_gc, vl_embed>**, bool, bool, int) > /gcc/gcc/cp/semantics.c:2728 > 0x9e9383 cp_parser_postfix_expression > /gcc/gcc/cp/parser.c:7549 This is discarded by finish_call_expr. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) > /gcc/gcc/cp/tree.c:728 > 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) > /gcc/gcc/cp/call.c:9747 > 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, tree_node*) > /gcc/gcc/cp/typeck.c:4025 > 0x95eb40 build_offset_ref_call_from_tree(tree_node*, vec<tree_node*, va_gc, vl_embed>**, int) > /gcc/gcc/cp/decl2.c:5292 > 0x9e9b1f cp_parser_postfix_expression > /gcc/gcc/cp/parser.c:7534 This is discarded by build_offset_ref_call_from_tree. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > /gcc/gcc/cp/tree.c:728 > 0x8b67c2 perform_direct_initialization_if_possible(tree_node*, tree_node*, bool, int) > /gcc/gcc/cp/call.c:12038 > 0xaba6a9 build_static_cast_1 > /gcc/gcc/cp/typeck.c:7607 > 0xabb500 build_static_cast(unsigned int, tree_node*, tree_node*, int) > /gcc/gcc/cp/typeck.c:7813 > 0x9e9f9e cp_parser_postfix_expression > /gcc/gcc/cp/parser.c:7049 This is discarded by build_static_cast. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0x97fb53 build_new_1 > /gcc/gcc/cp/init.c:3281 > 0x982382 build_new(unsigned int, vec<tree_node*, va_gc, vl_embed>**, tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, int) > /gcc/gcc/cp/init.c:3817 > 0x9f2107 cp_parser_new_expression > /gcc/gcc/cp/parser.c:8919 Discarded by build_new. > 0x6b3983 build_target_expr > /gcc/gcc/cp/tree.c:496 > 0xaa2b0f get_target_expr(tree_node*) > /gcc/gcc/cp/tree.c:899 > 0xaa2b0f stabilize_expr(tree_node*, tree_node**) > /gcc/gcc/cp/tree.c:5509 > 0xabfa2a cp_build_modify_expr(unsigned int, tree_node*, tree_code, tree_node*, int) > /gcc/gcc/cp/typeck.c:8736 > 0x8b7c91 build_new_op_1 > /gcc/gcc/cp/call.c:6537 > 0x8b809d build_new_op(op_location_t const&, tree_code, int, tree_node*, tree_node*, tree_node*, tree_node**, int) > /gcc/gcc/cp/call.c:6623 > 0xac0b04 build_x_modify_expr(unsigned int, tree_node*, tree_code, tree_node*, int) > /gcc/gcc/cp/typeck.c:8936 > 0x9ccafe cp_parser_assignment_expression > /gcc/gcc/cp/parser.c:10002 Discarded by build_x_modify_expr.
On Mon, 16 Nov 2020, Jason Merrill wrote: > On 11/13/20 10:43 AM, Patrick Palka wrote: > > On Thu, 12 Nov 2020, Jason Merrill wrote: > > > > > On 11/12/20 1:27 PM, Patrick Palka wrote: > > > > The atom_cache in normalize_atom relies on the assumption that two > > > > equivalent (templated) trees (in the sense of cp_tree_equal) must use > > > > the same template parameters (according to find_template_parameters). > > > > > > > > This assumption unfortunately doesn't always hold for TARGET_EXPRs, > > > > because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but > > > > find_template_parameters walks this target (and its DECL_CONTEXT). > > > > > > > > Hence two TARGET_EXPRs built by force_target_expr with the same > > > > initializer but under different settings of current_function_decl may > > > > compare equal according to cp_tree_equal, but find_template_parameters > > > > returns a different set of template parameters for them. This breaks > > > > the below testcase because during normalization we build two such > > > > TARGET_EXPRs (one under current_function_decl=f and another under =g), > > > > and then use the same ATOMIC_CONSTR for the two corresponding atoms, > > > > leading to a crash during satisfaction of g's associated constraints. > > > > > > > > This patch works around this assumption violation by removing the source > > > > of these templated TARGET_EXPRs. The relevant call to get_target_expr > > > > was > > > > added in r9-6043, but it seems it's no longer necessary (according to > > > > https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the > > > > call was added in order to avoid regressing on initlist109.C at the > > > > time). > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > > > trunk? > > > > > > OK. I wonder what else asserting !processing_template_decl in > > > build_target_expr would find... > > > > FWIW, testing exposed seven distinct paths that trigger such an assert, > > five of which go through build_cplus_new: > > Most of these are built as part of overload resolution and then thrown away, > only using the result for its type. But I wonder about the build_aggr_init > instance. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > > /gcc/gcc/cp/tree.c:728 > > 0x91712b force_rvalue(tree_node*, int) > > /gcc/gcc/cp/cvt.c:569 > > 0x8b8310 build_conditional_expr_1 > > /gcc/gcc/cp/call.c:5592 > > 0x8ba08c build_conditional_expr(op_location_t const&, tree_node*, > > tree_node*, tree_node*, int) > > /gcc/gcc/cp/call.c:5777 > > 0xaa70fb build_x_conditional_expr(unsigned int, tree_node*, tree_node*, > > tree_node*, int) > > /gcc/gcc/cp/typeck.c:7133 > > 0x9cc9fa cp_parser_assignment_expression > > /gcc/gcc/cp/parser.c:9964 > > This one gets discarded by build_x_conditional_expr. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > > /gcc/gcc/cp/tree.c:728 > > 0x97d185 expand_default_init > > /gcc/gcc/cp/init.c:1924 > > 0x97d185 expand_aggr_init_1 > > /gcc/gcc/cp/init.c:2101 > > 0x97f026 build_aggr_init(tree_node*, tree_node*, int, int) > > /gcc/gcc/cp/init.c:1835 > > 0x92c88d build_aggr_init_full_exprs > > /gcc/gcc/cp/decl.c:6696 > > 0x92c88d check_initializer > > /gcc/gcc/cp/decl.c:6857 > > 0x950982 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) > > /gcc/gcc/cp/decl.c:7699 > > 0x960c7e grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, > > bool, tree_node*, tree_node*) > > /gcc/gcc/cp/decl2.c:1000 > > 0xa02ceb cp_parser_member_declaration > > /gcc/gcc/cp/parser.c:25755 > > This one looks dubious. Which testcase? IIRC many constexpr testcases trigger this one, for instance g++.dg/cpp0x/constexpr-initlist9.C and g++.dg/cpp0x/constexpr-static6.C: gcc/testsuite/g++.dg/cpp0x/constexpr-initlist9.C: In function ‘void generate_sudoku(T)’: gcc/testsuite/g++.dg/cpp0x/constexpr-initlist9.C:34:50: internal compiler error: in build_target_expr, at cp/tree.c:495 34 | constexpr auto positions = make_grid_positions(); // fail | ^ 0x6b3c47 build_target_expr /home/patrick/code/gcc/gcc/cp/tree.c:495 0xa97248 build_cplus_new(tree_node*, tree_node*, int) /home/patrick/code/gcc/gcc/cp/tree.c:727 0x97eed5 expand_default_init /home/patrick/code/gcc/gcc/cp/init.c:1924 0x97eed5 expand_aggr_init_1 /home/patrick/code/gcc/gcc/cp/init.c:2101 0x980d76 build_aggr_init(tree_node*, tree_node*, int, int) /home/patrick/code/gcc/gcc/cp/init.c:1835 0x92e5cd build_aggr_init_full_exprs /home/patrick/code/gcc/gcc/cp/decl.c:6696 0x92e5cd check_initializer /home/patrick/code/gcc/gcc/cp/decl.c:6857 0x9525c2 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) /home/patrick/code/gcc/gcc/cp/decl.c:7699 0x9fcdf7 cp_parser_init_declarator /home/patrick/code/gcc/gcc/cp/parser.c:21362 gcc/testsuite/g++.dg/cpp0x/constexpr-static6.C:17:28: internal compiler error: in build_target_expr, at cp/tree.c:495 17 | constexpr static B t = B(); | ^ 0x6b3c47 build_target_expr /home/patrick/code/gcc/gcc/cp/tree.c:495 0xa97248 build_cplus_new(tree_node*, tree_node*, int) /home/patrick/code/gcc/gcc/cp/tree.c:727 0x97eed5 expand_default_init /home/patrick/code/gcc/gcc/cp/init.c:1924 0x97eed5 expand_aggr_init_1 /home/patrick/code/gcc/gcc/cp/init.c:2101 0x980d76 build_aggr_init(tree_node*, tree_node*, int, int) /home/patrick/code/gcc/gcc/cp/init.c:1835 0x92e5cd build_aggr_init_full_exprs /home/patrick/code/gcc/gcc/cp/decl.c:6696 0x92e5cd check_initializer /home/patrick/code/gcc/gcc/cp/decl.c:6857 0x9525c2 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) /home/patrick/code/gcc/gcc/cp/decl.c:7699 0x9629ce grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, bool, tree_node*, tree_node*) /home/patrick/code/gcc/gcc/cp/decl2.c:1000 > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) > > /gcc/gcc/cp/tree.c:728 > > 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) > > /gcc/gcc/cp/call.c:9747 > > 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, > > vl_embed>**, int, tree_node*) > > /gcc/gcc/cp/typeck.c:4025 > > 0xa75a30 finish_call_expr(tree_node*, vec<tree_node*, va_gc, vl_embed>**, > > bool, bool, int) > > /gcc/gcc/cp/semantics.c:2728 > > 0x9e9383 cp_parser_postfix_expression > > /gcc/gcc/cp/parser.c:7549 > > This is discarded by finish_call_expr. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) > > /gcc/gcc/cp/tree.c:728 > > 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) > > /gcc/gcc/cp/call.c:9747 > > 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, > > vl_embed>**, int, tree_node*) > > /gcc/gcc/cp/typeck.c:4025 > > 0x95eb40 build_offset_ref_call_from_tree(tree_node*, vec<tree_node*, va_gc, > > vl_embed>**, int) > > /gcc/gcc/cp/decl2.c:5292 > > 0x9e9b1f cp_parser_postfix_expression > > /gcc/gcc/cp/parser.c:7534 > > This is discarded by build_offset_ref_call_from_tree. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) > > /gcc/gcc/cp/tree.c:728 > > 0x8b67c2 perform_direct_initialization_if_possible(tree_node*, tree_node*, > > bool, int) > > /gcc/gcc/cp/call.c:12038 > > 0xaba6a9 build_static_cast_1 > > /gcc/gcc/cp/typeck.c:7607 > > 0xabb500 build_static_cast(unsigned int, tree_node*, tree_node*, int) > > /gcc/gcc/cp/typeck.c:7813 > > 0x9e9f9e cp_parser_postfix_expression > > /gcc/gcc/cp/parser.c:7049 > > This is discarded by build_static_cast. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0x97fb53 build_new_1 > > /gcc/gcc/cp/init.c:3281 > > 0x982382 build_new(unsigned int, vec<tree_node*, va_gc, vl_embed>**, > > tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, int) > > /gcc/gcc/cp/init.c:3817 > > 0x9f2107 cp_parser_new_expression > > /gcc/gcc/cp/parser.c:8919 > > Discarded by build_new. > > > 0x6b3983 build_target_expr > > /gcc/gcc/cp/tree.c:496 > > 0xaa2b0f get_target_expr(tree_node*) > > /gcc/gcc/cp/tree.c:899 > > 0xaa2b0f stabilize_expr(tree_node*, tree_node**) > > /gcc/gcc/cp/tree.c:5509 > > 0xabfa2a cp_build_modify_expr(unsigned int, tree_node*, tree_code, > > tree_node*, int) > > /gcc/gcc/cp/typeck.c:8736 > > 0x8b7c91 build_new_op_1 > > /gcc/gcc/cp/call.c:6537 > > 0x8b809d build_new_op(op_location_t const&, tree_code, int, tree_node*, > > tree_node*, tree_node*, tree_node**, int) > > /gcc/gcc/cp/call.c:6623 > > 0xac0b04 build_x_modify_expr(unsigned int, tree_node*, tree_code, > > tree_node*, int) > > /gcc/gcc/cp/typeck.c:8936 > > 0x9ccafe cp_parser_assignment_expression > > /gcc/gcc/cp/parser.c:10002 > > Discarded by build_x_modify_expr. > >
On 11/16/20 5:45 PM, Patrick Palka wrote: > On Mon, 16 Nov 2020, Jason Merrill wrote: > >> On 11/13/20 10:43 AM, Patrick Palka wrote: >>> On Thu, 12 Nov 2020, Jason Merrill wrote: >>> >>>> On 11/12/20 1:27 PM, Patrick Palka wrote: >>>>> The atom_cache in normalize_atom relies on the assumption that two >>>>> equivalent (templated) trees (in the sense of cp_tree_equal) must use >>>>> the same template parameters (according to find_template_parameters). >>>>> >>>>> This assumption unfortunately doesn't always hold for TARGET_EXPRs, >>>>> because cp_tree_equal ignores an artificial target of a TARGET_EXPR, but >>>>> find_template_parameters walks this target (and its DECL_CONTEXT). >>>>> >>>>> Hence two TARGET_EXPRs built by force_target_expr with the same >>>>> initializer but under different settings of current_function_decl may >>>>> compare equal according to cp_tree_equal, but find_template_parameters >>>>> returns a different set of template parameters for them. This breaks >>>>> the below testcase because during normalization we build two such >>>>> TARGET_EXPRs (one under current_function_decl=f and another under =g), >>>>> and then use the same ATOMIC_CONSTR for the two corresponding atoms, >>>>> leading to a crash during satisfaction of g's associated constraints. >>>>> >>>>> This patch works around this assumption violation by removing the source >>>>> of these templated TARGET_EXPRs. The relevant call to get_target_expr >>>>> was >>>>> added in r9-6043, but it seems it's no longer necessary (according to >>>>> https://gcc.gnu.org/pipermail/gcc-patches/2019-February/517323.html, the >>>>> call was added in order to avoid regressing on initlist109.C at the >>>>> time). >>>>> >>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>>>> trunk? >>>> >>>> OK. I wonder what else asserting !processing_template_decl in >>>> build_target_expr would find... >>> >>> FWIW, testing exposed seven distinct paths that trigger such an assert, >>> five of which go through build_cplus_new: >> >> Most of these are built as part of overload resolution and then thrown away, >> only using the result for its type. But I wonder about the build_aggr_init >> instance. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) >>> /gcc/gcc/cp/tree.c:728 >>> 0x91712b force_rvalue(tree_node*, int) >>> /gcc/gcc/cp/cvt.c:569 >>> 0x8b8310 build_conditional_expr_1 >>> /gcc/gcc/cp/call.c:5592 >>> 0x8ba08c build_conditional_expr(op_location_t const&, tree_node*, >>> tree_node*, tree_node*, int) >>> /gcc/gcc/cp/call.c:5777 >>> 0xaa70fb build_x_conditional_expr(unsigned int, tree_node*, tree_node*, >>> tree_node*, int) >>> /gcc/gcc/cp/typeck.c:7133 >>> 0x9cc9fa cp_parser_assignment_expression >>> /gcc/gcc/cp/parser.c:9964 >> >> This one gets discarded by build_x_conditional_expr. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) >>> /gcc/gcc/cp/tree.c:728 >>> 0x97d185 expand_default_init >>> /gcc/gcc/cp/init.c:1924 >>> 0x97d185 expand_aggr_init_1 >>> /gcc/gcc/cp/init.c:2101 >>> 0x97f026 build_aggr_init(tree_node*, tree_node*, int, int) >>> /gcc/gcc/cp/init.c:1835 >>> 0x92c88d build_aggr_init_full_exprs >>> /gcc/gcc/cp/decl.c:6696 >>> 0x92c88d check_initializer >>> /gcc/gcc/cp/decl.c:6857 >>> 0x950982 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) >>> /gcc/gcc/cp/decl.c:7699 >>> 0x960c7e grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, >>> bool, tree_node*, tree_node*) >>> /gcc/gcc/cp/decl2.c:1000 >>> 0xa02ceb cp_parser_member_declaration >>> /gcc/gcc/cp/parser.c:25755 >> >> This one looks dubious. Which testcase? > > IIRC many constexpr testcases trigger this one, for instance > g++.dg/cpp0x/constexpr-initlist9.C and g++.dg/cpp0x/constexpr-static6.C: > > gcc/testsuite/g++.dg/cpp0x/constexpr-initlist9.C: In function ‘void generate_sudoku(T)’: > gcc/testsuite/g++.dg/cpp0x/constexpr-initlist9.C:34:50: internal compiler error: in build_target_expr, at cp/tree.c:495 > 34 | constexpr auto positions = make_grid_positions(); // fail > | ^ > 0x6b3c47 build_target_expr > /home/patrick/code/gcc/gcc/cp/tree.c:495 > 0xa97248 build_cplus_new(tree_node*, tree_node*, int) > /home/patrick/code/gcc/gcc/cp/tree.c:727 > 0x97eed5 expand_default_init > /home/patrick/code/gcc/gcc/cp/init.c:1924 > 0x97eed5 expand_aggr_init_1 > /home/patrick/code/gcc/gcc/cp/init.c:2101 > 0x980d76 build_aggr_init(tree_node*, tree_node*, int, int) > /home/patrick/code/gcc/gcc/cp/init.c:1835 > 0x92e5cd build_aggr_init_full_exprs > /home/patrick/code/gcc/gcc/cp/decl.c:6696 > 0x92e5cd check_initializer > /home/patrick/code/gcc/gcc/cp/decl.c:6857 > 0x9525c2 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) > /home/patrick/code/gcc/gcc/cp/decl.c:7699 > 0x9fcdf7 cp_parser_init_declarator > /home/patrick/code/gcc/gcc/cp/parser.c:21362 > > > gcc/testsuite/g++.dg/cpp0x/constexpr-static6.C:17:28: internal compiler error: in build_target_expr, at cp/tree.c:495 > 17 | constexpr static B t = B(); > | ^ > 0x6b3c47 build_target_expr > /home/patrick/code/gcc/gcc/cp/tree.c:495 > 0xa97248 build_cplus_new(tree_node*, tree_node*, int) > /home/patrick/code/gcc/gcc/cp/tree.c:727 > 0x97eed5 expand_default_init > /home/patrick/code/gcc/gcc/cp/init.c:1924 > 0x97eed5 expand_aggr_init_1 > /home/patrick/code/gcc/gcc/cp/init.c:2101 > 0x980d76 build_aggr_init(tree_node*, tree_node*, int, int) > /home/patrick/code/gcc/gcc/cp/init.c:1835 > 0x92e5cd build_aggr_init_full_exprs > /home/patrick/code/gcc/gcc/cp/decl.c:6696 > 0x92e5cd check_initializer > /home/patrick/code/gcc/gcc/cp/decl.c:6857 > 0x9525c2 cp_finish_decl(tree_node*, tree_node*, bool, tree_node*, int) > /home/patrick/code/gcc/gcc/cp/decl.c:7699 > 0x9629ce grokfield(cp_declarator const*, cp_decl_specifier_seq*, tree_node*, bool, tree_node*, tree_node*) > /home/patrick/code/gcc/gcc/cp/decl2.c:1000 Thanks. This is part of trying to produce a real value for a non-dependent constexpr variable; the TARGET_EXPR gets melted down before long, so it's OK. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) >>> /gcc/gcc/cp/tree.c:728 >>> 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) >>> /gcc/gcc/cp/call.c:9747 >>> 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, >>> vl_embed>**, int, tree_node*) >>> /gcc/gcc/cp/typeck.c:4025 >>> 0xa75a30 finish_call_expr(tree_node*, vec<tree_node*, va_gc, vl_embed>**, >>> bool, bool, int) >>> /gcc/gcc/cp/semantics.c:2728 >>> 0x9e9383 cp_parser_postfix_expression >>> /gcc/gcc/cp/parser.c:7549 >> >> This is discarded by finish_call_expr. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xa94ee8 build_cplus_new(tree_node*, tree_node*, int) >>> /gcc/gcc/cp/tree.c:728 >>> 0x8a0ed5 build_cxx_call(tree_node*, int, tree_node**, int, tree_node*) >>> /gcc/gcc/cp/call.c:9747 >>> 0xabed0b cp_build_function_call_vec(tree_node*, vec<tree_node*, va_gc, >>> vl_embed>**, int, tree_node*) >>> /gcc/gcc/cp/typeck.c:4025 >>> 0x95eb40 build_offset_ref_call_from_tree(tree_node*, vec<tree_node*, va_gc, >>> vl_embed>**, int) >>> /gcc/gcc/cp/decl2.c:5292 >>> 0x9e9b1f cp_parser_postfix_expression >>> /gcc/gcc/cp/parser.c:7534 >> >> This is discarded by build_offset_ref_call_from_tree. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xa94ec8 build_cplus_new(tree_node*, tree_node*, int) >>> /gcc/gcc/cp/tree.c:728 >>> 0x8b67c2 perform_direct_initialization_if_possible(tree_node*, tree_node*, >>> bool, int) >>> /gcc/gcc/cp/call.c:12038 >>> 0xaba6a9 build_static_cast_1 >>> /gcc/gcc/cp/typeck.c:7607 >>> 0xabb500 build_static_cast(unsigned int, tree_node*, tree_node*, int) >>> /gcc/gcc/cp/typeck.c:7813 >>> 0x9e9f9e cp_parser_postfix_expression >>> /gcc/gcc/cp/parser.c:7049 >> >> This is discarded by build_static_cast. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0x97fb53 build_new_1 >>> /gcc/gcc/cp/init.c:3281 >>> 0x982382 build_new(unsigned int, vec<tree_node*, va_gc, vl_embed>**, >>> tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, int, int) >>> /gcc/gcc/cp/init.c:3817 >>> 0x9f2107 cp_parser_new_expression >>> /gcc/gcc/cp/parser.c:8919 >> >> Discarded by build_new. >> >>> 0x6b3983 build_target_expr >>> /gcc/gcc/cp/tree.c:496 >>> 0xaa2b0f get_target_expr(tree_node*) >>> /gcc/gcc/cp/tree.c:899 >>> 0xaa2b0f stabilize_expr(tree_node*, tree_node**) >>> /gcc/gcc/cp/tree.c:5509 >>> 0xabfa2a cp_build_modify_expr(unsigned int, tree_node*, tree_code, >>> tree_node*, int) >>> /gcc/gcc/cp/typeck.c:8736 >>> 0x8b7c91 build_new_op_1 >>> /gcc/gcc/cp/call.c:6537 >>> 0x8b809d build_new_op(op_location_t const&, tree_code, int, tree_node*, >>> tree_node*, tree_node*, tree_node**, int) >>> /gcc/gcc/cp/call.c:6623 >>> 0xac0b04 build_x_modify_expr(unsigned int, tree_node*, tree_code, >>> tree_node*, int) >>> /gcc/gcc/cp/typeck.c:8936 >>> 0x9ccafe cp_parser_assignment_expression >>> /gcc/gcc/cp/parser.c:10002 >> >> Discarded by build_x_modify_expr. >>
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 33d715edaec..172286922e7 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -3006,12 +3006,7 @@ finish_compound_literal (tree type, tree compound_literal, /* If we're in a template, return the original compound literal. */ if (orig_cl) - { - if (!VECTOR_TYPE_P (type)) - return get_target_expr_sfinae (orig_cl, complain); - else - return orig_cl; - } + return orig_cl; if (TREE_CODE (compound_literal) == CONSTRUCTOR) { diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C new file mode 100644 index 00000000000..837855ce8ac --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-decltype3.C @@ -0,0 +1,15 @@ +// { dg-do compile { target c++20 } } + +template <class T> concept C = requires(T t) { t; }; + +template <class T> using A = decltype((T{}, int{})); + +template <class T> concept D = C<A<T>>; + +template <class T, class U> void f() requires D<T>; +template <class T> void g() requires D<T>; + +void h() { + f<int, int>(); + g<int>(); +}