[pushed] c++: corresponding object parms [PR113191]

Message ID 20240111220148.1575375-1-jason@redhat.com
State New
Headers
Series [pushed] c++: corresponding object parms [PR113191] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 warning Patch is already merged

Commit Message

Jason Merrill Jan. 11, 2024, 10:01 p.m. UTC
  Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

As discussed, our handling of corresponding object parameters needed to
handle the using-declaration case better.  And I took the opportunity to
share code between the add_method and cand_parms_match uses.

This patch specifically doesn't compare reversed parameters, but a follow-up
patch will.

	PR c++/113191

gcc/cp/ChangeLog:

	* class.cc (xobj_iobj_parameters_correspond): Add context parm.
	(object_parms_correspond): Factor out of...
	(add_method): ...here.
	* method.cc (defaulted_late_check): Use it.
	* call.cc (class_of_implicit_object): New.
	(object_parms_correspond): Overload taking two candidates.
	(cand_parms_match): Use it.
	(joust): Check reversed before comparing constraints.
	* cp-tree.h (object_parms_correspond): Declare.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-memfun4.C: New test.
---
 gcc/cp/cp-tree.h                              |   1 +
 gcc/cp/call.cc                                |  84 ++++++++----
 gcc/cp/class.cc                               | 121 ++++++++++--------
 gcc/cp/method.cc                              |  16 +--
 gcc/testsuite/g++.dg/cpp2a/concepts-memfun4.C |  97 ++++++++++++++
 5 files changed, 228 insertions(+), 91 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-memfun4.C


base-commit: 9bac1d7839f129f93f159c27adaf472ee3ab23a2
  

Comments

Jason Merrill Jan. 12, 2024, 2:11 p.m. UTC | #1
On 1/11/24 17:01, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> -- 8< --
> 
> As discussed, our handling of corresponding object parameters needed to
> handle the using-declaration case better.  And I took the opportunity to
> share code between the add_method and cand_parms_match uses.
> 
> This patch specifically doesn't compare reversed parameters, but a follow-up
> patch will.

Thus.
  

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f3f265a3db8..83009fc837c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6853,6 +6853,7 @@  extern bool is_empty_base_ref			(tree);
 extern tree build_vtbl_ref			(tree, tree);
 extern tree build_vfn_ref			(tree, tree);
 extern tree get_vtable_decl			(tree, int);
+extern bool object_parms_correspond		(tree, tree, tree);
 extern bool add_method				(tree, tree, bool);
 extern tree declared_access			(tree);
 extern bool maybe_push_used_methods		(tree);
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 191664ee227..6f024b8abc3 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -12685,6 +12685,51 @@  joust_maybe_elide_copy (z_candidate *cand)
   return false;
 }
 
+/* Return the class that CAND's implicit object parameter refers to.  */
+
+static tree
+class_of_implicit_object (z_candidate *cand)
+{
+  if (!DECL_IOBJ_MEMBER_FUNCTION_P (cand->fn))
+    return NULL_TREE;
+
+  /* "For conversion functions that are implicit object member functions,
+     the function is considered to be a member of the class of the implied
+     object argument for the purpose of defining the type of the implicit
+     object parameter."  */
+  if (DECL_CONV_FN_P (cand->fn))
+    return TYPE_MAIN_VARIANT (TREE_TYPE (cand->first_arg));
+
+  /* "For non-conversion functions that are implicit object member
+     functions nominated by a using-declaration in a derived class, the
+     function is considered to be a member of the derived class for the
+     purpose of defining the type of the implicit object parameter."
+
+     That derived class is reflected in the conversion_path binfo.  */
+  return BINFO_TYPE (cand->conversion_path);
+}
+
+/* True if candidates C1 and C2 have corresponding object parameters per
+   [basic.scope.scope].  */
+
+static bool
+object_parms_correspond (z_candidate *c1, z_candidate *c2)
+{
+  tree context = class_of_implicit_object (c1);
+  tree ctx2 = class_of_implicit_object (c2);
+  if (!ctx2)
+    /* Leave context as is. */;
+  else if (!context)
+    context = ctx2;
+  else if (context != ctx2)
+    /* This can't happen for normal function calls, since it means finding
+       functions in multiple bases which would fail with an ambiguous lookup,
+       but it can occur with reversed operators.  */
+    return false;
+
+  return object_parms_correspond (c1->fn, c2->fn, context);
+}
+
 /* True if the defining declarations of the two candidates have equivalent
    parameters.  */
 
@@ -12712,35 +12757,25 @@  cand_parms_match (z_candidate *c1, z_candidate *c2)
     }
   tree parms1 = TYPE_ARG_TYPES (TREE_TYPE (fn1));
   tree parms2 = TYPE_ARG_TYPES (TREE_TYPE (fn2));
-  auto skip_parms = [](tree fn, tree parms){
-      if (DECL_XOBJ_MEMBER_FUNCTION_P (fn))
-	return TREE_CHAIN (parms);
-      else
-	return skip_artificial_parms_for (fn, parms);
-    };
   if (!(DECL_FUNCTION_MEMBER_P (fn1)
 	&& DECL_FUNCTION_MEMBER_P (fn2)))
     /* Early escape.  */;
-  else if ((DECL_STATIC_FUNCTION_P (fn1)
-	    != DECL_STATIC_FUNCTION_P (fn2)))
+
+  /* CWG2789 is not adequate, it should specify corresponding object
+     parameters, not same typed object parameters.  */
+  else if (!object_parms_correspond (c1, c2))
+    return false;
+  else
     {
-      /* Ignore 'this' when comparing the parameters of a static member
-	 function with those of a non-static one.  */
-      parms1 = skip_parms (fn1, parms1);
-      parms2 = skip_parms (fn2, parms2);
-    }
-  else if ((DECL_XOBJ_MEMBER_FUNCTION_P (fn1)
-	    || DECL_XOBJ_MEMBER_FUNCTION_P (fn2))
-	   && (DECL_IOBJ_MEMBER_FUNCTION_P (fn1)
-	       || DECL_IOBJ_MEMBER_FUNCTION_P (fn2)))
-    {
-      bool xobj_iobj_parameters_correspond (tree, tree);
-      /* CWG2789 is not adequate, it should specify corresponding object
-	 parameters, not same typed object parameters.  */
-      if (!xobj_iobj_parameters_correspond (fn1, fn2))
-	return false;
       /* We just compared the object parameters, if they don't correspond
-	 we already return false.  */
+	 we already returned false.  */
+      auto skip_parms = [] (tree fn, tree parms)
+	{
+	  if (DECL_XOBJ_MEMBER_FUNCTION_P (fn))
+	    return TREE_CHAIN (parms);
+	  else
+	    return skip_artificial_parms_for (fn, parms);
+	};
       parms1 = skip_parms (fn1, parms1);
       parms2 = skip_parms (fn2, parms2);
     }
@@ -13104,6 +13139,7 @@  joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn,
 
   if (flag_concepts && DECL_P (cand1->fn) && DECL_P (cand2->fn)
       && !cand1->template_decl && !cand2->template_decl
+      && cand1->reversed () == cand2->reversed ()
       && cand_parms_match (cand1, cand2))
     {
       winner = more_constrained (cand1->fn, cand2->fn);
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index e5e609badf3..3374756fb9a 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -1020,15 +1020,11 @@  modify_vtable_entry (tree t,
 
 
 /* Check if the object parameters of an xobj and iobj member function
-   correspond.  This function assumes that the iobj parameter has been
-   correctly adjusted when the function is introduced by a using declaration
-   per [over.match.funcs.general.4].
+   correspond.  CONTEXT is the class that an implicit object parameter
+   refers to.  */
 
-   ??? But it isn't, that's only considered at overload resolution time.
-   cand_parms_match will probably need to check cand->conversion_path.  */
-
-bool
-xobj_iobj_parameters_correspond (tree fn1, tree fn2)
+static bool
+xobj_iobj_parameters_correspond (tree fn1, tree fn2, tree context)
 {
   gcc_assert (DECL_IOBJ_MEMBER_FUNCTION_P (fn1)
 	      || DECL_IOBJ_MEMBER_FUNCTION_P (fn2));
@@ -1042,11 +1038,6 @@  xobj_iobj_parameters_correspond (tree fn1, tree fn2)
 
   tree iobj_fn = DECL_IOBJ_MEMBER_FUNCTION_P (fn1) ? fn1 : fn2;
   tree iobj_fn_type = TREE_TYPE (iobj_fn);
-  /* Will work for a pointer or reference param type.  So this will continue
-     to work even if we change how the object parameter of an iobj member
-     function is represented.  */
-  tree iobj_param_type
-    = TREE_TYPE (TREE_VALUE (TYPE_ARG_TYPES (iobj_fn_type)));
 
   /* If the iobj member function was introduced with a using declaration, the
      type of its object parameter is considered to be that of the class it was
@@ -1116,7 +1107,7 @@  xobj_iobj_parameters_correspond (tree fn1, tree fn2)
      check for that case.  */
 
   if (!same_type_ignoring_top_level_qualifiers_p
-      (iobj_param_type, non_reference (xobj_param)))
+      (context, non_reference (xobj_param)))
     return false;
 
   /* We don't get to bail yet even if we have a by-value xobj parameter,
@@ -1214,6 +1205,63 @@  xobj_iobj_parameters_correspond (tree fn1, tree fn2)
   return true;
 }
 
+/* True if FN and METHOD have corresponding object parms per
+   [basic.scope.scope], or if one of them is a static member function (which
+   are considered to have an object parm that corresponds to any other).
+   CONTEXT is the class that an implicit object member function is considered
+   to be a member of for the purpose of this comparison, per
+   [over.match.funcs].  */
+
+bool
+object_parms_correspond (tree fn, tree method, tree context)
+{
+  tree fn_type = TREE_TYPE (fn);
+  tree method_type = TREE_TYPE (method);
+
+  /* Compare the quals on the 'this' parm.  Don't compare
+     the whole types, as used functions are treated as
+     coming from the using class in overload resolution.  */
+  if (DECL_IOBJ_MEMBER_FUNCTION_P (fn)
+      && DECL_IOBJ_MEMBER_FUNCTION_P (method))
+    {
+      /* Either both or neither need to be ref-qualified for
+	 differing quals to allow overloading.  */
+      if ((FUNCTION_REF_QUALIFIED (fn_type)
+	   == FUNCTION_REF_QUALIFIED (method_type))
+	  && (type_memfn_quals (fn_type) != type_memfn_quals (method_type)
+	      || type_memfn_rqual (fn_type) != type_memfn_rqual (method_type)))
+	return false;
+      return true;
+    }
+  /* Treat a static member function as corresponding to any object parm.  */
+  else if (DECL_STATIC_FUNCTION_P (fn) || DECL_STATIC_FUNCTION_P (method))
+    return true;
+  /* Handle special correspondence rules for xobj vs xobj and xobj vs iobj
+     member function declarations.
+     We don't worry about static member functions here.  */
+  else if (DECL_XOBJ_MEMBER_FUNCTION_P (fn)
+	   && DECL_XOBJ_MEMBER_FUNCTION_P (method))
+    {
+      auto get_object_param = [] (tree fn)
+	{
+	  return TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fn)));
+	};
+      /* We skip the object parameter below, check it here instead of
+	 making changes to that code.  */
+      tree fn_param = get_object_param (fn);
+      tree method_param = get_object_param (method);
+      if (!same_type_p (fn_param, method_param))
+	return false;
+    }
+  else if (DECL_XOBJ_MEMBER_FUNCTION_P (fn)
+	   || DECL_XOBJ_MEMBER_FUNCTION_P (method))
+    return xobj_iobj_parameters_correspond (fn, method, context);
+  else
+    gcc_unreachable ();
+
+  return true;
+}
+
 /* Add method METHOD to class TYPE.  If VIA_USING indicates whether
    METHOD is being injected via a using_decl.  Returns true if the
    method could be added to the method vec.  */
@@ -1268,51 +1316,12 @@  add_method (tree type, tree method, bool via_using)
 	 functions in the derived class override and/or hide member
 	 functions with the same name and parameter types in a base
 	 class (rather than conflicting).  */
+      if (!object_parms_correspond (fn, method, type))
+	continue;
+
       tree fn_type = TREE_TYPE (fn);
       tree method_type = TREE_TYPE (method);
 
-      /* Compare the quals on the 'this' parm.  Don't compare
-	 the whole types, as used functions are treated as
-	 coming from the using class in overload resolution.  */
-      if (DECL_IOBJ_MEMBER_FUNCTION_P (fn)
-	  && DECL_IOBJ_MEMBER_FUNCTION_P (method)
-	  /* Either both or neither need to be ref-qualified for
-	     differing quals to allow overloading.  */
-	  && (FUNCTION_REF_QUALIFIED (fn_type)
-	      == FUNCTION_REF_QUALIFIED (method_type))
-	  && (type_memfn_quals (fn_type) != type_memfn_quals (method_type)
-	      || type_memfn_rqual (fn_type) != type_memfn_rqual (method_type)))
-	  continue;
-
-      /* Handle special correspondence rules for xobj vs xobj and xobj vs iobj
-	 member function declarations.
-	 We don't worry about static member functions here.  */
-      if ((!DECL_XOBJ_MEMBER_FUNCTION_P (fn)
-	   && !DECL_XOBJ_MEMBER_FUNCTION_P (method))
-	  || DECL_STATIC_FUNCTION_P (fn) || DECL_STATIC_FUNCTION_P (method))
-	/* Early escape.  */;
-      else if (DECL_XOBJ_MEMBER_FUNCTION_P (fn)
-	       && DECL_XOBJ_MEMBER_FUNCTION_P (method))
-	{
-	  auto get_object_param = [](tree fn){
-	    return TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fn)));
-	  };
-	  /* We skip the object parameter below, check it here instead of
-	     making changes to that code.  */
-	  tree fn_param = get_object_param (fn);
-	  tree method_param = get_object_param (method);
-	  if (!same_type_p (fn_param, method_param))
-	    continue;
-	}
-      else if (DECL_XOBJ_MEMBER_FUNCTION_P (fn)
-	       || DECL_XOBJ_MEMBER_FUNCTION_P (method))
-	{
-	  if (!xobj_iobj_parameters_correspond (fn, method))
-	    continue;
-	}
-      else
-	gcc_unreachable ();
-
       tree real_fn = fn;
       tree real_method = method;
 
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 6a9f03eb8f3..d49e5a565e8 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3395,24 +3395,18 @@  defaulted_late_check (tree fn)
       {
 	tree fn_obj_ref_type = TREE_VALUE (fn_parms);
 	/* We can't default xobj operators with an xobj parameter that is not
-	   an lvalue reference.  */
+	   an lvalue reference, even if it would correspond.  */
 	if (!TYPE_REF_P (fn_obj_ref_type)
-	    || TYPE_REF_IS_RVALUE (fn_obj_ref_type))
-	  return false;
-	/* If implicit_fn's object parameter is not a pointer, something is not
-	   right.  */
-	gcc_assert (TYPE_PTR_P (TREE_VALUE (implicit_fn_parms)));
-	/* Strip the reference/pointer off each object parameter before
-	   comparing them.  */
-	if (!same_type_p (TREE_TYPE (fn_obj_ref_type),
-			  TREE_TYPE (TREE_VALUE (implicit_fn_parms))))
+	    || TYPE_REF_IS_RVALUE (fn_obj_ref_type)
+	    || !object_parms_correspond (fn, implicit_fn,
+					 DECL_CONTEXT (implicit_fn)))
 	  return false;
 	/* We just compared the object parameters, skip over them before
 	   passing to compparms.  */
 	fn_parms = TREE_CHAIN (fn_parms);
 	implicit_fn_parms = TREE_CHAIN (implicit_fn_parms);
       }
-    return compparms(fn_parms, implicit_fn_parms);
+    return compparms (fn_parms, implicit_fn_parms);
   };
 
   if (!same_type_p (TREE_TYPE (TREE_TYPE (fn)),
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-memfun4.C b/gcc/testsuite/g++.dg/cpp2a/concepts-memfun4.C
new file mode 100644
index 00000000000..2fa661dbe96
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-memfun4.C
@@ -0,0 +1,97 @@ 
+// PR c++/113191
+// { dg-do compile { target c++23 } }
+
+template<typename> struct S;
+
+template<typename T = void>
+struct B {
+  constexpr int f() const requires true { return 5; }
+  constexpr operator int () const requires true { return 5; }
+  constexpr int g(this S<T>&&) requires true { return 5; }
+  constexpr int h() requires true { return 5; }
+};
+
+template<typename = void>
+struct S : B<> {
+  using B::f;
+  using B::g;
+  using B::h;
+  constexpr int f() const { return 10; }
+  constexpr operator int () const { return 10; }
+  constexpr int g() { return 10; }
+  constexpr int h(this S&&) { return 10; }
+};
+
+// implicit object parms match, B::f is more constrained
+static_assert(S<>{}.f() == 5);
+static_assert(S<>{}.g() == 5);
+static_assert(S<>{}.h() == 5);
+
+template <typename = void>
+struct C {
+  constexpr int f() const { return 15; }
+  constexpr operator int () const { return 15; }
+};
+
+template <typename = void>
+struct S2: B<>, C<> { };
+
+// implicit object parms for conversion functions are all considered to be from
+// the class of the object argument
+static_assert(S2<>{} == 5);
+
+// ambiguous lookup, so we never actually compare the candidates
+// if we did, implicit object parms don't match due to different classes
+// so constraints aren't considered and it would still be ambiguous
+static_assert(S2<>{}.f() == 5);	// { dg-error "ambiguous" }
+
+template <typename = void>
+struct S3 : B<> {
+  using B::f;
+  constexpr int f() volatile { return 10; }
+};
+
+// implicit object parms don't match due to different cv-quals
+static_assert(S3<>{}.f() == 5);	// { dg-error "ambiguous" }
+
+template <typename = void>
+struct S4 : B<> {
+  using B::f;
+  constexpr int f() const & { return 10; }
+};
+
+// no ref-qual matches any ref-qual
+static_assert(S4<>{}.f() == 5);
+
+template <typename = void>
+struct C2 {
+  constexpr operator int () volatile { return 15; }
+};
+
+template <typename = void>
+struct S5: B<>, C2<> { };
+
+// implicit object parms don't match due to different cv-quals
+static_assert(S5<>{} == 5);	// { dg-error "ambiguous" }
+
+namespace N1 {
+  template <class = void> struct B;
+
+  template <class = void>
+  struct A {
+    constexpr bool operator==(B<>&) { return true; }
+  };
+
+  template <class>
+  struct B {
+    constexpr bool operator==(A<>&) requires true { return false; }
+  };
+
+  A<> a;
+  B<> b;
+  // when comparing the A op== to the reversed B op==, we don't compare
+  // constraints and so fall through to the tiebreaker that chooses the
+  // non-reversed candidate.
+  // ??? shouldn't we compare constraints?
+  static_assert (a == b);
+}