[pushed] c++: Tuple of self-dependent classes [PR96926]

Message ID 20210219022206.3972395-1-jason@redhat.com
State New
Headers show
Series
  • [pushed] c++: Tuple of self-dependent classes [PR96926]
Related show

Commit Message

Kewen.Lin via Gcc-patches Feb. 19, 2021, 2:22 a.m.
When compiling this testcase, trying to resolve the initialization for the
tuple member ends up recursively considering the same set of tuple
constructor overloads, and since two of them separately depend on
is_constructible, the one we try second fails to instantiate
is_constructible because we're still in the middle of instantiating it the
first time.

Fixed by implementing an optimization that someone suggested we were already
doing: if we see a non-template candidate that is a perfect match for all
arguments, we can skip considering template candidates at all.  It would be
enough to do this only when LOOKUP_DEFAULTED, but it shouldn't hurt in other
cases.

Tested x86_64-pc-linux-gnu, applying to trunk.

gcc/cp/ChangeLog:

	PR c++/96926
	* call.c (perfect_conversion_p): New.
	(perfect_candidate_p): New.
	(add_candidates): Ignore templates after a perfect non-template.

gcc/testsuite/ChangeLog:

	PR c++/96926
	* g++.dg/cpp0x/overload4.C: New test.
---
 gcc/cp/call.c                          |  90 +++++++++++--
 gcc/testsuite/g++.dg/cpp0x/overload4.C | 174 +++++++++++++++++++++++++
 2 files changed, 252 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/overload4.C


base-commit: a1541628df83eb690954f426d56d268fb57f1af6
-- 
2.27.0

Comments

Kewen.Lin via Gcc-patches Feb. 19, 2021, 5 a.m. | #1
On 2/18/21 9:22 PM, Jason Merrill wrote:
> When compiling this testcase, trying to resolve the initialization for the

> tuple member ends up recursively considering the same set of tuple

> constructor overloads, and since two of them separately depend on

> is_constructible, the one we try second fails to instantiate

> is_constructible because we're still in the middle of instantiating it the

> first time.

> 

> Fixed by implementing an optimization that someone suggested we were already

> doing: if we see a non-template candidate that is a perfect match for all

> arguments, we can skip considering template candidates at all.  It would be

> enough to do this only when LOOKUP_DEFAULTED, but it shouldn't hurt in other

> cases.
From d909ead68214042e9876a8df136d0835273d4b86 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>

Date: Thu, 18 Feb 2021 21:27:37 -0500
Subject: [PATCH] c++: Tweak PR969626 patch
To: gcc-patches@gcc.gnu.org

It occurred to me that other types of conversions use rvaluedness_matches_p,
but those uses don't affect overload resolution, so we shouldn't look at the
flag for them.  Fixing that made decltype64.C compile successfully, because
the non-template candidate was a perfect match, so we now wouldn't consider
the broken template.  Changing the argument to const& makes it no longer a
perfect match (because of the added const), so we again get the infinite
recursion.

This illustrates the limited nature of this optimization/recursion break; it
works for most copy/move constructors because the constructor we're looking
for is almost always a perfect match.  If it happens to help improve compile
time for other calls, that's just a bonus.

gcc/cp/ChangeLog:

	PR c++/96926
	* call.c (perfect_conversion_p): Limit rvalueness
	test to reference bindings.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/decltype64.C: Change argument to const&.
---
 gcc/cp/call.c                           | 14 ++++++++------
 gcc/testsuite/g++.dg/cpp0x/decltype64.C |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index bc369c68c5a..0ba0e19ae08 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5864,12 +5864,14 @@ perfect_conversion_p (conversion *conv)
 {
   if (CONVERSION_RANK (conv) != cr_identity)
     return false;
-  if (!conv->rvaluedness_matches_p)
-    return false;
-  if (conv->kind == ck_ref_bind
-      && !same_type_p (TREE_TYPE (conv->type),
-		       next_conversion (conv)->type))
-    return false;
+  if (conv->kind == ck_ref_bind)
+    {
+      if (!conv->rvaluedness_matches_p)
+	return false;
+      if (!same_type_p (TREE_TYPE (conv->type),
+			next_conversion (conv)->type))
+	return false;
+    }
   return true;
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/decltype64.C b/gcc/testsuite/g++.dg/cpp0x/decltype64.C
index 46d18594c94..0cd614cceeb 100644
--- a/gcc/testsuite/g++.dg/cpp0x/decltype64.C
+++ b/gcc/testsuite/g++.dg/cpp0x/decltype64.C
@@ -5,7 +5,7 @@ template<int I>
 struct index
 {};
 
-constexpr int recursive_impl(index<0u>)
+constexpr int recursive_impl(const index<0u>&)
 {
   return 0;
 }
-- 
2.27.0

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 186feef6fe3..bc369c68c5a 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5853,6 +5853,47 @@  prep_operand (tree operand)
   return operand;
 }
 
+/* True iff CONV represents a conversion sequence which no other can be better
+   than under [over.ics.rank]: in other words, a "conversion" to the exact same
+   type (including binding to a reference to the same type).  This is stronger
+   than the standard's "identity" category, which also includes reference
+   bindings that add cv-qualifiers or change rvalueness.  */
+
+static bool
+perfect_conversion_p (conversion *conv)
+{
+  if (CONVERSION_RANK (conv) != cr_identity)
+    return false;
+  if (!conv->rvaluedness_matches_p)
+    return false;
+  if (conv->kind == ck_ref_bind
+      && !same_type_p (TREE_TYPE (conv->type),
+		       next_conversion (conv)->type))
+    return false;
+  return true;
+}
+
+/* True if CAND represents a perfect match, i.e. all perfect conversions, so no
+   other candidate can be a better match.  Since the template/non-template
+   tiebreaker comes immediately after the conversion comparison in
+   [over.match.best], a perfect non-template candidate is better than all
+   templates.  */
+
+static bool
+perfect_candidate_p (z_candidate *cand)
+{
+  if (cand->viable < 1)
+    return false;
+  int len = cand->num_convs;
+  for (int i = 0; i < len; ++i)
+    if (!perfect_conversion_p (cand->convs[i]))
+      return false;
+  if (conversion *conv = cand->second_conv)
+    if (!perfect_conversion_p (conv))
+      return false;
+  return true;
+}
+
 /* Add each of the viable functions in FNS (a FUNCTION_DECL or
    OVERLOAD) to the CANDIDATES, returning an updated list of
    CANDIDATES.  The ARGS are the arguments provided to the call;
@@ -5920,6 +5961,18 @@  add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
     /* Delay creating the implicit this parameter until it is needed.  */
     non_static_args = NULL;
 
+  /* If there's a non-template perfect match, we don't need to consider
+     templates.  So check non-templates first.  This optimization is only
+     really needed for the defaulted copy constructor of tuple and the like
+     (96926), but it seems like we might as well enable it more generally.  */
+  bool seen_perfect = false;
+  enum { templates, non_templates, either } which = either;
+  if (template_only)
+    which = templates;
+  else /*if (flags & LOOKUP_DEFAULTED)*/
+    which = non_templates;
+
+ again:
   for (lkp_iterator iter (fns); iter; ++iter)
     {
       fn = *iter;
@@ -5928,6 +5981,10 @@  add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 	continue;
       if (check_list_ctor && !is_list_ctor (fn))
 	continue;
+      if (which == templates && TREE_CODE (fn) != TEMPLATE_DECL)
+	continue;
+      if (which == non_templates && TREE_CODE (fn) == TEMPLATE_DECL)
+	continue;
 
       tree fn_first_arg = NULL_TREE;
       const vec<tree, va_gc> *fn_args = args;
@@ -5967,7 +6024,7 @@  add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 				fn,
 				ctype,
 				explicit_targs,
-				fn_first_arg, 
+				fn_first_arg,
 				fn_args,
 				return_type,
 				access_path,
@@ -5975,17 +6032,26 @@  add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 				flags,
 				strict,
 				complain);
-      else if (!template_only)
-	add_function_candidate (candidates,
-				fn,
-				ctype,
-				fn_first_arg,
-				fn_args,
-				access_path,
-				conversion_path,
-				flags,
-				NULL,
-				complain);
+      else
+	{
+	  add_function_candidate (candidates,
+				  fn,
+				  ctype,
+				  fn_first_arg,
+				  fn_args,
+				  access_path,
+				  conversion_path,
+				  flags,
+				  NULL,
+				  complain);
+	  if (perfect_candidate_p (*candidates))
+	    seen_perfect = true;
+	}
+    }
+  if (which == non_templates && !seen_perfect)
+    {
+      which = templates;
+      goto again;
     }
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/overload4.C b/gcc/testsuite/g++.dg/cpp0x/overload4.C
new file mode 100644
index 00000000000..b2f8eb1ba02
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/overload4.C
@@ -0,0 +1,174 @@ 
+// PR c++/96926
+// { dg-do compile { target c++11 } }
+
+namespace std
+{
+  template<typename _Tp, _Tp __v>
+    struct integral_constant
+    {
+      static constexpr _Tp value = __v;
+      typedef integral_constant<_Tp, __v> type;
+    };
+
+  template<typename _Tp, _Tp __v>
+    constexpr _Tp integral_constant<_Tp, __v>::value;
+
+  typedef integral_constant<bool, true> true_type;
+  typedef integral_constant<bool, false> false_type;
+
+  template<bool __v>
+    using bool_constant = integral_constant<bool, __v>;
+
+  template<bool, typename, typename>
+    struct conditional;
+
+  template<typename...>
+    struct __and_;
+
+  template<>
+    struct __and_<>
+    : public true_type
+    { };
+
+  template<typename _B1>
+    struct __and_<_B1>
+    : public _B1
+    { };
+
+  template<typename _B1, typename _B2>
+    struct __and_<_B1, _B2>
+    : public conditional<_B1::value, _B2, _B1>::type
+    { };
+
+  template<typename _B1, typename _B2, typename _B3, typename... _Bn>
+    struct __and_<_B1, _B2, _B3, _Bn...>
+    : public conditional<_B1::value, __and_<_B2, _B3, _Bn...>, _B1>::type
+    { };
+
+  template<typename _Tp, typename... _Args>
+    struct is_constructible
+    : public bool_constant<__is_constructible(_Tp, _Args...)>
+    {
+    };
+
+  template<bool, typename _Tp = void>
+    struct enable_if
+    { };
+
+  template<typename _Tp>
+    struct enable_if<true, _Tp>
+    { typedef _Tp type; };
+
+  template<bool _Cond, typename _Tp = void>
+    using __enable_if_t = typename enable_if<_Cond, _Tp>::type;
+
+  template<bool _Cond, typename _Iftrue, typename _Iffalse>
+    struct conditional
+    { typedef _Iftrue type; };
+
+
+  template<typename _Iftrue, typename _Iffalse>
+    struct conditional<false, _Iftrue, _Iffalse>
+    { typedef _Iffalse type; };
+
+
+  template<bool, typename... _Types>
+    struct _TupleConstraints
+    {
+      template<typename... _UTypes>
+        static constexpr bool __is_implicitly_constructible()
+        {
+          // is_constructible is incomplete here, but only when
+          // it is also instantiated in __is_explicitly_constructible 
+          return __and_<is_constructible<_Types, _UTypes>...,
+                 true_type
+                   >::value;
+        }
+
+      template<typename... _UTypes>
+        static constexpr bool __is_explicitly_constructible()
+        {
+#if FIX
+          return false;
+#else
+          return __and_<is_constructible<_Types, _UTypes>...,
+                 false_type
+                   >::value;
+#endif
+        }
+    };
+
+  template<typename... _Elements>
+    class tuple
+    {
+      template<bool _Cond>
+        using _TCC = _TupleConstraints<_Cond, _Elements...>;
+
+      template<bool _Cond, typename... _Args>
+        using _ImplicitCtor = __enable_if_t<
+        _TCC<_Cond>::template __is_implicitly_constructible<_Args...>(),
+        bool>;
+
+      template<bool _Cond, typename... _Args>
+        using _ExplicitCtor = __enable_if_t<
+        _TCC<_Cond>::template __is_explicitly_constructible<_Args...>(),
+        bool>;
+
+    public:
+
+      template<bool _NotEmpty = true,
+        _ImplicitCtor<_NotEmpty, const _Elements&...> = true>
+          constexpr
+          tuple(const _Elements&... __elements)
+          { }
+
+      template<bool _NotEmpty = true,
+        _ExplicitCtor<_NotEmpty, const _Elements&...> = false>
+          explicit constexpr
+          tuple(const _Elements&... __elements)
+          { }
+    };
+}
+
+// first example
+
+template <typename SessionT>
+struct SomeQuery {
+    SessionT& session_;
+    SomeQuery(SessionT& session) : session_(session) {}
+};
+
+template <typename SessionT>
+struct Handler {
+    std::tuple<SomeQuery<SessionT>> queries_;
+    Handler(SessionT& session) : queries_(session) {}
+};
+
+struct Session {
+    Handler<Session> handler_;
+    Session() : handler_{*this} {}
+};
+
+int main() {
+    Session session;
+}
+static_assert(std::is_constructible<SomeQuery<Session>, const SomeQuery<Session>&>::value, "");
+
+// second example
+
+template <typename T>
+class DependsOnT
+{
+public:
+    DependsOnT(T&) {}
+};
+
+class Test
+{
+public:
+    Test() : test_{*this} {}
+
+private:
+    std::tuple<DependsOnT<Test>> test_;
+};
+static_assert(std::is_constructible<DependsOnT<Test>, const DependsOnT<Test>&>::value, "");