[gcc r13-1755] c++: shortcut bad reference binding [PR94894]

[gcc r13-1755] c++: shortcut bad reference binding [PR94894]

[Patrick Palka]

https://gcc.gnu.org/g:68f37670eff0b872ce5dfd382c8d8f3206bdfc27

commit r13-1755-g68f37670eff0b872ce5dfd382c8d8f3206bdfc27
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue Jul 19 14:04:13 2022 -0400

    c++: shortcut bad reference binding [PR94894]
    
    In case of l/rvalue or cv-qual mismatch during reference binding, we
    try to give more helpful diagnostics by computing a bad conversion that
    allows the mismatch.  But in doing so, we may end up considering and
    instantiating a conversion function that could induce a hard error and
    in turn cause us to reject otherwise valid code.  We could just give up
    on producing a better diagnostic here, but ideally we'd preserve the
    better diagnostics for invalid code while avoiding unnecessary template
    instantiations for valid code.
    
    To that end, this patch adapts the bad conversion shortcutting mechanism
    from r12-3346-g47543e5f9d1fc5 to additionally handle this situation.
    The main observation from there is that during overload resolution, if we
    know we have a strictly viable candidate then we don't need to distinguish
    between an unviable and non-strictly viable candidate.  Thus we don't
    need to distinguish between an invalid and bad conversion either, which
    is what this patch exploits.  Of course, we don't know whether we have a
    strictly viable candidate until after the fact, so we still need to
    remember when we deferred distinguishing between an invalid and bad
    conversion.  This patch adds a special conversion kind ck_deferred_bad
    for this purpose.
    
            PR c++/94894
            PR c++/105766
            PR c++/106201
    
    gcc/cp/ChangeLog:
    
            * call.cc (enum conversion_kind): Add ck_deferred_bad enumerator.
            (has_next): Return false for it.
            (reference_binding): Return a ck_deferred_bad conversion instead
            of an actual bad conversion when LOOKUP_SHORTCUT_BAD_CONVS is set.
            Remove now obsolete early exit for the incomplete TO case.
            (implicit_conversion_1): Don't mask out LOOKUP_SHORTCUT_BAD_CONVS.
            (add_function_candidate): Set LOOKUP_SHORTCUT_BAD_CONVS iff
            shortcut_bad_convs.
            (missing_conversion_p): Also return true for a ck_deferred_bad
            conversion.
            * cp-tree.h (LOOKUP_SHORTCUT_BAD_CONVS): Define.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/conversion/ref8.C: New test.
            * g++.dg/conversion/ref9.C: New test.

Diff:
---
 gcc/cp/call.cc                         | 92 ++++++++++++++++++++++------------
 gcc/cp/cp-tree.h                       |  5 ++
 gcc/testsuite/g++.dg/conversion/ref8.C | 22 ++++++++
 gcc/testsuite/g++.dg/conversion/ref9.C | 21 ++++++++
 4 files changed, 108 insertions(+), 32 deletions(-)

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 191c68cdcfd..01a7be10077 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -59,7 +59,13 @@ enum conversion_kind {
   ck_ambig,
   ck_list,
   ck_aggr,
-  ck_rvalue
+  ck_rvalue,
+  /* When LOOKUP_SHORTCUT_BAD_CONVS is set, we may return a conversion of
+     this kind whenever we know the true conversion is either bad or outright
+     invalid, but we don't want to attempt to compute the bad conversion (for
+     sake of avoiding unnecessary instantiation).  bad_p should always be set
+     for these.  */
+  ck_deferred_bad,
 };
 
 /* The rank of the conversion.  Order of the enumerals matters; better
@@ -775,7 +781,8 @@ has_next (conversion_kind code)
   return !(code == ck_identity
 	   || code == ck_ambig
 	   || code == ck_list
-	   || code == ck_aggr);
+	   || code == ck_aggr
+	   || code == ck_deferred_bad);
 }
 
 static conversion *
@@ -1912,18 +1919,38 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
      difference in top-level cv-qualification is subsumed by the
      initialization itself and does not constitute a conversion.  */
 
+  bool maybe_valid_p = true;
+
   /* [dcl.init.ref]
 
      Otherwise, the reference shall be an lvalue reference to a
      non-volatile const type, or the reference shall be an rvalue
-     reference.
+     reference.  */
+  if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto))
+    maybe_valid_p = false;
 
-     We try below to treat this as a bad conversion to improve diagnostics,
-     but if TO is an incomplete class, we need to reject this conversion
-     now to avoid unnecessary instantiation.  */
-  if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto)
-      && !COMPLETE_TYPE_P (to))
-    return NULL;
+  /* [dcl.init.ref]
+
+     Otherwise, a temporary of type "cv1 T1" is created and
+     initialized from the initializer expression using the rules for a
+     non-reference copy initialization.  If T1 is reference-related to
+     T2, cv1 must be the same cv-qualification as, or greater
+     cv-qualification than, cv2; otherwise, the program is ill-formed.  */
+  if (related_p && !at_least_as_qualified_p (to, from))
+    maybe_valid_p = false;
+
+  /* We try below to treat an invalid reference binding as a bad conversion
+     to improve diagnostics, but doing so may cause otherwise unnecessary
+     instantiations that can lead to a hard error.  So during the first pass
+     of overload resolution wherein we shortcut bad conversions, instead just
+     produce a special conversion indicating a second pass is necessary if
+     there's no strictly viable candidate.  */
+  if (!maybe_valid_p && (flags & LOOKUP_SHORTCUT_BAD_CONVS))
+    {
+      conv = alloc_conversion (ck_deferred_bad);
+      conv->bad_p = true;
+      return conv;
+    }
 
   /* We're generating a temporary now, but don't bind any more in the
      conversion (specifically, don't slice the temporary returned by a
@@ -1967,7 +1994,9 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
 				   sflags, complain);
 	    if (!new_second)
 	      return NULL;
-	    return merge_conversion_sequences (t, new_second);
+	    conv = merge_conversion_sequences (t, new_second);
+	    gcc_assert (maybe_valid_p || conv->bad_p);
+	    return conv;
 	  }
     }
 
@@ -1976,24 +2005,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
      creation of a temporary.  */
   conv->need_temporary_p = true;
   conv->rvaluedness_matches_p = TYPE_REF_IS_RVALUE (rto);
-
-  /* [dcl.init.ref]
-
-     Otherwise, the reference shall be an lvalue reference to a
-     non-volatile const type, or the reference shall be an rvalue
-     reference.  */
-  if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto))
-    conv->bad_p = true;
-
-  /* [dcl.init.ref]
-
-     Otherwise, a temporary of type "cv1 T1" is created and
-     initialized from the initializer expression using the rules for a
-     non-reference copy initialization.  If T1 is reference-related to
-     T2, cv1 must be the same cv-qualification as, or greater
-     cv-qualification than, cv2; otherwise, the program is ill-formed.  */
-  if (related_p && !at_least_as_qualified_p (to, from))
-    conv->bad_p = true;
+  conv->bad_p |= !maybe_valid_p;
 
   return conv;
 }
@@ -2015,7 +2027,8 @@ implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p,
      resolution, or after we've chosen one.  */
   flags &= (LOOKUP_ONLYCONVERTING|LOOKUP_NO_CONVERSION|LOOKUP_COPY_PARM
 	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_PREFER_RVALUE
-	    |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL);
+	    |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL
+	    |LOOKUP_SHORTCUT_BAD_CONVS);
 
   /* FIXME: actually we don't want warnings either, but we can't just
      have 'complain &= ~(tf_warning|tf_error)' because it would cause
@@ -2433,6 +2446,11 @@ add_function_candidate (struct z_candidate **candidates,
   if (! viable)
     goto out;
 
+  if (shortcut_bad_convs)
+    flags |= LOOKUP_SHORTCUT_BAD_CONVS;
+  else
+    flags &= ~LOOKUP_SHORTCUT_BAD_CONVS;
+
   /* Third, for F to be a viable function, there shall exist for each
      argument an implicit conversion sequence that converts that argument
      to the corresponding parameter of F.  */
@@ -6038,14 +6056,24 @@ perfect_candidate_p (z_candidate *cand)
   return true;
 }
 
-/* True iff one of CAND's argument conversions is NULL.  */
+/* True iff one of CAND's argument conversions is missing.  */
 
 static bool
 missing_conversion_p (const z_candidate *cand)
 {
   for (unsigned i = 0; i < cand->num_convs; ++i)
-    if (!cand->convs[i])
-      return true;
+    {
+      conversion *conv = cand->convs[i];
+      if (!conv)
+	return true;
+      if (conv->kind == ck_deferred_bad)
+	{
+	  /* We don't know whether this conversion is outright invalid or
+	     just bad, so conservatively assume it's missing.  */
+	  gcc_checking_assert (conv->bad_p);
+	  return true;
+	}
+    }
   return false;
 }
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index cf51c39dc90..3278b4114bd 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5877,6 +5877,11 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
 #define LOOKUP_REVERSED (LOOKUP_REWRITTEN << 1)
 /* We're initializing an aggregate from a parenthesized list of values.  */
 #define LOOKUP_AGGREGATE_PAREN_INIT (LOOKUP_REVERSED << 1)
+/* We're computing conversions as part of a first pass of overload resolution
+   wherein we don't try to distinguish an unviable candidate from a
+   non-strictly viable candidate and thus can avoid computing unnecessary
+   bad conversions.  */
+#define LOOKUP_SHORTCUT_BAD_CONVS (LOOKUP_AGGREGATE_PAREN_INIT << 1)
 
 /* These flags are used by the conversion code.
    CONV_IMPLICIT   :  Perform implicit conversions (standard and user-defined).
diff --git a/gcc/testsuite/g++.dg/conversion/ref8.C b/gcc/testsuite/g++.dg/conversion/ref8.C
new file mode 100644
index 00000000000..0dd29f751ca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ref8.C
@@ -0,0 +1,22 @@
+// PR c++/105766
+// { dg-do compile { target c++20 } }
+
+template<class T>
+struct baz {
+  baz() = default;
+  baz(int) requires requires { T(0); };
+};
+
+struct foo;
+
+struct bar {
+  bar() = default;
+  bar(foo&);
+  bar(int);
+};
+
+struct foo {
+  baz<bar> m_bars;
+};
+
+foo a;
diff --git a/gcc/testsuite/g++.dg/conversion/ref9.C b/gcc/testsuite/g++.dg/conversion/ref9.C
new file mode 100644
index 00000000000..e6dfc03cd7f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ref9.C
@@ -0,0 +1,21 @@
+// PR c++/106201
+// { dg-do compile { target c++11 } }
+
+struct A {
+  template<class T, class = decltype(f(*(T*)nullptr))>
+  A(const T&);
+};
+
+struct B {
+  template<class T> B(const T&);
+};
+
+void f(A&);
+void f(B);
+
+struct C { };
+
+int main() {
+  C c;
+  f(c);
+}