From 3eecc1db4c691a87ef4a229d059aa863066d9a1c Mon Sep 17 00:00:00 2001 From: Patrick Palka Date: Wed, 23 Jun 2021 08:24:34 -0400 Subject: [PATCH] c++: CTAD and deduction guide selection [PR86439] During CTAD, we select the best viable deduction guide using build_new_function_call, which performs overload resolution on the set of candidate guides and then forms a call to the guide. As the PR points out, this latter step is unnecessary and occasionally incorrect since a call to the selected guide may be ill-formed, or forming the call may have side effects such as prematurely deducing the type of a {}. So this patch introduces a specialized subroutine based on build_new_function_call that stops short of building a call to the selected function, and makes do_class_deduction use this subroutine instead. And since a call is no longer built, do_class_deduction doesn't need to set tf_decltype or cp_unevaluated_operand anymore. This change causes us to reject some container CTAD examples in the libstdc++ testsuite due to deduction failure for {}, which AFAICT is the correct behavior. Previously in e.g. the first removed example std::map{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, {}}, the type of the {} would get deduced to less as a side effect of forming a call to the chosen guide template, typename _Allocator = allocator>> map(initializer_list>, _Compare = _Compare(), _Allocator = _Allocator()) -> map<_Key, _Tp, _Compare, _Allocator>; which made later overload resolution for the constructor call unambiguous. Now, the type of the {} remains undeduced until constructor overload resolution, and we complain about ambiguity for the two equally good constructor candidates map(initializer_list, const _Compare& = _Compare(), const allocator_type& = allocator_type()) map(initializer_list, const allocator_type&). This patch fixes these problematic container CTAD examples by giving the {} an appropriate concrete type. Two of these adjusted CTAD examples (one for std::set and one for std::multiset) end up triggering an unrelated CTAD bug on trunk, PR101174, so these two adjusted examples are commented out for now. PR c++/86439 gcc/cp/ChangeLog: * call.c (print_error_for_call_failure): Constify 'args' parameter. (perform_dguide_overload_resolution): Define. * cp-tree.h: (perform_dguide_overload_resolution): Declare. * pt.c (do_class_deduction): Use perform_dguide_overload_resolution instead of build_new_function_call. Don't use tf_decltype or set cp_unevaluated_operand. Remove unnecessary NULL_TREE tests. libstdc++-v3/ChangeLog: * testsuite/23_containers/map/cons/deduction.cc: Replace ambiguous CTAD examples. * testsuite/23_containers/multimap/cons/deduction.cc: Likewise. * testsuite/23_containers/multiset/cons/deduction.cc: Likewise. Mention one of the replaced examples is broken due to PR101174. * testsuite/23_containers/set/cons/deduction.cc: Likewise. * testsuite/23_containers/unordered_map/cons/deduction.cc: Replace ambiguous CTAD examples. * testsuite/23_containers/unordered_multimap/cons/deduction.cc: Likewise. * testsuite/23_containers/unordered_multiset/cons/deduction.cc: Likewise. * testsuite/23_containers/unordered_set/cons/deduction.cc: Likewise. gcc/testsuite/ChangeLog: * g++.dg/cpp1z/class-deduction88.C: New test. * g++.dg/cpp1z/class-deduction89.C: New test. * g++.dg/cpp1z/class-deduction90.C: New test. --- gcc/cp/call.c | 36 ++++++++++++++++++- gcc/cp/cp-tree.h | 2 ++ gcc/cp/pt.c | 41 ++++++++-------------- gcc/testsuite/g++.dg/cpp1z/class-deduction88.C | 18 ++++++++++ gcc/testsuite/g++.dg/cpp1z/class-deduction89.C | 15 ++++++++ gcc/testsuite/g++.dg/cpp1z/class-deduction90.C | 16 +++++++++ .../testsuite/23_containers/map/cons/deduction.cc | 8 ++--- .../23_containers/multimap/cons/deduction.cc | 8 ++--- .../23_containers/multiset/cons/deduction.cc | 8 +++-- .../testsuite/23_containers/set/cons/deduction.cc | 8 +++-- .../23_containers/unordered_map/cons/deduction.cc | 17 +++++++-- .../unordered_multimap/cons/deduction.cc | 17 +++++++-- .../unordered_multiset/cons/deduction.cc | 14 ++++++-- .../23_containers/unordered_set/cons/deduction.cc | 14 ++++++-- 14 files changed, 170 insertions(+), 52 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction88.C create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction89.C create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction90.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 9f03534..aafc7ac 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -4629,7 +4629,7 @@ perform_overload_resolution (tree fn, functions. */ static void -print_error_for_call_failure (tree fn, vec *args, +print_error_for_call_failure (tree fn, const vec *args, struct z_candidate *candidates) { tree targs = NULL_TREE; @@ -4654,6 +4654,40 @@ print_error_for_call_failure (tree fn, vec *args, print_z_candidates (loc, candidates); } +/* Perform overload resolution on the set of deduction guides DGUIDES + using ARGS. Returns the selected deduction guide, or error_mark_node + if overload resolution fails. */ + +tree +perform_dguide_overload_resolution (tree dguides, const vec *args, + tsubst_flags_t complain) +{ + z_candidate *candidates; + bool any_viable_p; + tree result; + + gcc_assert (deduction_guide_p (OVL_FIRST (dguides))); + + /* Get the high-water mark for the CONVERSION_OBSTACK. */ + void *p = conversion_obstack_alloc (0); + + z_candidate *cand = perform_overload_resolution (dguides, args, &candidates, + &any_viable_p, complain); + if (!cand) + { + if (complain & tf_error) + print_error_for_call_failure (dguides, args, candidates); + result = error_mark_node; + } + else + result = cand->fn; + + /* Free all the conversions we allocated. */ + obstack_free (&conversion_obstack, p); + + return result; +} + /* Return an expression for a call to FN (a namespace-scope function, or a static member function) with the ARGS. This may change ARGS. */ diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 36f99cc..6f71371 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6437,6 +6437,8 @@ extern void complain_about_bad_argument (location_t arg_loc, tree from_type, tree to_type, tree fndecl, int parmnum); extern void maybe_inform_about_fndecl_for_bogus_argument_init (tree, int); +extern tree perform_dguide_overload_resolution (tree, const vec *, + tsubst_flags_t); /* A class for recording information about access failures (e.g. private diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 15947b2..732fb40 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -29382,7 +29382,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init, if (tree guide = maybe_aggr_guide (tmpl, init, args)) cands = lookup_add (guide, cands); - tree call = error_mark_node; + tree fndecl = error_mark_node; /* If this is list-initialization and the class has a list constructor, first try deducing from the list as a single argument, as [over.match.list]. */ @@ -29396,11 +29396,9 @@ do_class_deduction (tree ptype, tree tmpl, tree init, } if (list_cands) { - ++cp_unevaluated_operand; - call = build_new_function_call (list_cands, &args, tf_decltype); - --cp_unevaluated_operand; + fndecl = perform_dguide_overload_resolution (list_cands, args, tf_none); - if (call == error_mark_node) + if (fndecl == error_mark_node) { /* That didn't work, now try treating the list as a sequence of arguments. */ @@ -29416,31 +29414,22 @@ do_class_deduction (tree ptype, tree tmpl, tree init, "user-declared constructors", type); return error_mark_node; } - else if (!cands && call == error_mark_node) + else if (!cands && fndecl == error_mark_node) { error ("cannot deduce template arguments of %qT, as it has no viable " "deduction guides", type); return error_mark_node; } - if (call == error_mark_node) - { - ++cp_unevaluated_operand; - call = build_new_function_call (cands, &args, tf_decltype); - --cp_unevaluated_operand; - } + if (fndecl == error_mark_node) + fndecl = perform_dguide_overload_resolution (cands, args, tf_none); - if (call == error_mark_node) + if (fndecl == error_mark_node) { if (complain & tf_warning_or_error) { error ("class template argument deduction failed:"); - - ++cp_unevaluated_operand; - call = build_new_function_call (cands, &args, - complain | tf_decltype); - --cp_unevaluated_operand; - + perform_dguide_overload_resolution (cands, args, complain); if (elided) inform (input_location, "explicit deduction guides not considered " "for copy-initialization"); @@ -29451,8 +29440,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init, constructor is chosen, the initialization is ill-formed. */ else if (flags & LOOKUP_ONLYCONVERTING) { - tree fndecl = cp_get_callee_fndecl_nofold (call); - if (fndecl && DECL_NONCONVERTING_P (fndecl)) + if (DECL_NONCONVERTING_P (fndecl)) { if (complain & tf_warning_or_error) { @@ -29470,12 +29458,10 @@ do_class_deduction (tree ptype, tree tmpl, tree init, /* If CTAD succeeded but the type doesn't have any explicit deduction guides, this deduction might not be what the user intended. */ - if (call != error_mark_node && !any_dguides_p) + if (fndecl != error_mark_node && !any_dguides_p) { - tree fndecl = cp_get_callee_fndecl_nofold (call); - if (fndecl != NULL_TREE - && (!DECL_IN_SYSTEM_HEADER (fndecl) - || global_dc->dc_warn_system_headers) + if ((!DECL_IN_SYSTEM_HEADER (fndecl) + || global_dc->dc_warn_system_headers) && warning (OPT_Wctad_maybe_unsupported, "%qT may not intend to support class template argument " "deduction", type)) @@ -29483,7 +29469,8 @@ do_class_deduction (tree ptype, tree tmpl, tree init, "warning"); } - return cp_build_qualified_type (TREE_TYPE (call), cp_type_quals (ptype)); + return cp_build_qualified_type (TREE_TYPE (TREE_TYPE (fndecl)), + cp_type_quals (ptype)); } /* Replace occurrences of 'auto' in TYPE with the appropriate type deduced diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction88.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction88.C new file mode 100644 index 0000000..be38fed --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction88.C @@ -0,0 +1,18 @@ +// PR c++/86439 +// { dg-do compile { target c++17 } } + +struct NC { + NC() = default; + NC(NC const&) = delete; + NC& operator=(NC const&) = delete; +}; + +template +struct C { + C(NC const&); +}; + +C(NC) -> C<0>; + +NC nc; +C c(nc); diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction89.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction89.C new file mode 100644 index 0000000..dd89857 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction89.C @@ -0,0 +1,15 @@ +// PR c++/86439 +// { dg-do compile { target c++17 } } + +struct B { }; +struct C { }; + +template +struct A { + A(T, B); +}; + +template +A(T, C) -> A; + +A a(0, {}); diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction90.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction90.C new file mode 100644 index 0000000..8b93193 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction90.C @@ -0,0 +1,16 @@ +// PR c++/86439 +// { dg-do compile { target c++17 } } + +struct less { }; +struct allocator { }; + +template +struct A { + A(T, U); + A(T, V); +}; + +template +A(T, U) -> A; + +A a(0, {}); // { dg-error "ambiguous" } diff --git a/libstdc++-v3/testsuite/23_containers/map/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/map/cons/deduction.cc index e9628c4..e72033c 100644 --- a/libstdc++-v3/testsuite/23_containers/map/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/map/cons/deduction.cc @@ -40,7 +40,7 @@ static_assert(std::is_same_v< */ static_assert(std::is_same_v< - decltype(std::map{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, {}}), + decltype(std::map{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, std::less{}}), std::map>); /* This is not deducible, ambiguous candidates: @@ -92,7 +92,7 @@ void f() static_assert(std::is_same_v< decltype(std::map(x.begin(), x.end(), - {})), + std::less{})), std::map>); static_assert(std::is_same_v< @@ -145,7 +145,7 @@ void g() static_assert(std::is_same_v< decltype(std::map(x.begin(), x.end(), - {})), + std::less{})), std::map>); static_assert(std::is_same_v< @@ -195,7 +195,7 @@ void h() static_assert(std::is_same_v< decltype(std::map(x.begin(), x.end(), - {})), + std::less{})), std::map>); static_assert(std::is_same_v< diff --git a/libstdc++-v3/testsuite/23_containers/multimap/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/multimap/cons/deduction.cc index 791cc96..ffc7502 100644 --- a/libstdc++-v3/testsuite/23_containers/multimap/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/multimap/cons/deduction.cc @@ -42,7 +42,7 @@ static_assert(std::is_same_v< static_assert(std::is_same_v< decltype(std::multimap{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, - {}}), + std::less{}}), std::multimap>); static_assert(std::is_same_v< @@ -77,7 +77,7 @@ void f() static_assert(std::is_same_v< decltype(std::multimap(x.begin(), x.end(), - {})), + std::less{})), std::multimap>); static_assert(std::is_same_v< @@ -119,7 +119,7 @@ void g() static_assert(std::is_same_v< decltype(std::multimap(x.begin(), x.end(), - {})), + std::less{})), std::multimap>); static_assert(std::is_same_v< @@ -158,7 +158,7 @@ void h() static_assert(std::is_same_v< decltype(std::multimap(x.begin(), x.end(), - {})), + std::less{})), std::multimap>); static_assert(std::is_same_v< diff --git a/libstdc++-v3/testsuite/23_containers/multiset/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/multiset/cons/deduction.cc index ad12755..a4ccc6f 100644 --- a/libstdc++-v3/testsuite/23_containers/multiset/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/multiset/cons/deduction.cc @@ -19,9 +19,11 @@ static_assert(std::is_same_v< decltype(std::multiset{{1, 2, 3}, std::less{}, {}}), std::multiset>); +/* FIXME: GCC 12 rejects this due to PR c++/101174 static_assert(std::is_same_v< - decltype(std::multiset{{1, 2, 3}, {}}), + decltype(std::multiset{{1, 2, 3}, std::less{}}), std::multiset>); +*/ static_assert(std::is_same_v< decltype(std::multiset{{1, 2, 3}, SimpleAllocator{}}), @@ -52,7 +54,7 @@ void f() static_assert(std::is_same_v< decltype(std::multiset(x.begin(), x.end(), - {})), + std::less{})), std::multiset>); static_assert(std::is_same_v< @@ -103,7 +105,7 @@ void g() static_assert(std::is_same_v< decltype(std::multiset(x.begin(), x.end(), - {})), + std::less{})), std::multiset>); static_assert(std::is_same_v< diff --git a/libstdc++-v3/testsuite/23_containers/set/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/set/cons/deduction.cc index 89a2c43..0ae4c2a 100644 --- a/libstdc++-v3/testsuite/23_containers/set/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/set/cons/deduction.cc @@ -20,10 +20,12 @@ static_assert(std::is_same_v< std::less{}, {}}), std::set>); +/* FIXME: GCC 12 rejects this due to PR c++/101174 static_assert(std::is_same_v< decltype(std::set{{1, 2, 3}, - {}}), + std::less{}}), std::set>); +*/ static_assert(std::is_same_v< decltype(std::set{{1, 2, 3}, @@ -58,7 +60,7 @@ void f() static_assert(std::is_same_v< decltype(std::set(x.begin(), x.end(), - {})), + std::less{})), std::set>); static_assert(std::is_same_v< @@ -104,7 +106,7 @@ void g() static_assert(std::is_same_v< decltype(std::set(x.begin(), x.end(), - {})), + std::less{})), std::set>); static_assert(std::is_same_v< diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/deduction.cc index d8489b2..0785447 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/cons/deduction.cc @@ -24,7 +24,13 @@ static_assert(std::is_same_v< static_assert(std::is_same_v< decltype(std::unordered_map{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), + std::unordered_map>); + +static_assert(std::is_same_v< + decltype(std::unordered_map{{std::pair{1, 2.0}, + {2, 3.0}, {3, 4.0}}, + {}, std::hash{}, std::allocator>{}}), std::unordered_map>); static_assert(std::is_same_v< @@ -59,9 +65,14 @@ void f() static_assert(std::is_same_v< decltype(std::unordered_map{x.begin(), x.end(), - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), std::unordered_map>); - + + static_assert(std::is_same_v< + decltype(std::unordered_map{x.begin(), x.end(), + {}, std::hash{}, std::allocator>{}}), + std::unordered_map>); + static_assert(std::is_same_v< decltype(std::unordered_map(x.begin(), x.end(), {})), diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multimap/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/unordered_multimap/cons/deduction.cc index 13f54d4..d8a6f51 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multimap/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multimap/cons/deduction.cc @@ -18,7 +18,13 @@ static_assert(std::is_same_v< static_assert(std::is_same_v< decltype(std::unordered_multimap{{std::pair{1, 2.0}, {2, 3.0}, {3, 4.0}}, - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), + std::unordered_multimap>); + +static_assert(std::is_same_v< + decltype(std::unordered_multimap{{std::pair{1, 2.0}, + {2, 3.0}, {3, 4.0}}, + {}, std::hash{}, std::allocator>{}}), std::unordered_multimap>); static_assert(std::is_same_v< @@ -68,9 +74,14 @@ void f() static_assert(std::is_same_v< decltype(std::unordered_multimap{x.begin(), x.end(), - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), std::unordered_multimap>); - + + static_assert(std::is_same_v< + decltype(std::unordered_multimap{x.begin(), x.end(), + {}, std::hash{}, std::allocator>{}}), + std::unordered_multimap>); + static_assert(std::is_same_v< decltype(std::unordered_multimap(x.begin(), x.end(), {})), diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/cons/deduction.cc index 1850237..25c2715 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/cons/deduction.cc @@ -11,7 +11,12 @@ static_assert(std::is_same_v< static_assert(std::is_same_v< decltype(std::unordered_multiset{{1, 2, 3}, - 0, std::hash{}, {}}), + 0, std::hash{}, std::equal_to{}}), + std::unordered_multiset>); + +static_assert(std::is_same_v< + decltype(std::unordered_multiset{{1, 2, 3}, + 0, std::hash{}, std::allocator{}}), std::unordered_multiset>); static_assert(std::is_same_v< @@ -78,7 +83,12 @@ void f() static_assert(std::is_same_v< decltype(std::unordered_multiset{x.begin(), x.end(), - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), + std::unordered_multiset>); + + static_assert(std::is_same_v< + decltype(std::unordered_multiset{x.begin(), x.end(), + {}, std::hash{}, std::allocator{}}), std::unordered_multiset>); static_assert(std::is_same_v< diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/deduction.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/deduction.cc index a745dce..b8c45d2 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/cons/deduction.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/cons/deduction.cc @@ -11,7 +11,12 @@ static_assert(std::is_same_v< static_assert(std::is_same_v< decltype(std::unordered_set{{1, 2, 3}, - 0, std::hash{}, {}}), + 0, std::hash{}, std::equal_to{}}), + std::unordered_set>); + +static_assert(std::is_same_v< + decltype(std::unordered_set{{1, 2, 3}, + 0, std::hash{}, std::allocator{}}), std::unordered_set>); static_assert(std::is_same_v< @@ -73,7 +78,12 @@ void f() static_assert(std::is_same_v< decltype(std::unordered_set{x.begin(), x.end(), - {}, std::hash{}, {}}), + {}, std::hash{}, std::equal_to{}}), + std::unordered_set>); + + static_assert(std::is_same_v< + decltype(std::unordered_set{x.begin(), x.end(), + {}, std::hash{}, std::allocator{}}), std::unordered_set>); static_assert(std::is_same_v< -- 2.7.4