[PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[Jason Merrill]

Tested x86_64-pc-linux-gnu.  Are the library test changes OK?  A reduced
example of the issue is at https://godbolt.org/z/cPxrcnKjG

-- 8< --

Looks like we implemented option 1 (skip the object parameter) for CWG532
before the issue was resolved, and never updated to the final resolution of
option 2 (model it as a reference).  More recently CWG2445 extended this
handling to static member functions; I think that's wrong, and have
opened CWG2834 to address that and how explicit object member functions
interact with it.

The FIXME comments are to guide how the explicit object member function
support should change the uses of DECL_NONSTATIC_MEMBER_FUNCTION_P.

The library testsuite changes are to make partial ordering work again
between the generic operator- in the testcase and
_Pointer_adapter::operator-.

	DR 532
	PR c++/53499

gcc/cp/ChangeLog:

	* pt.cc (more_specialized_fn): Fix object parameter handling.

gcc/testsuite/ChangeLog:

	* g++.dg/template/partial-order4.C: New test.
	* g++.dg/template/spec26.C: Adjust for CWG532.

libstdc++-v3/ChangeLog:

	* testsuite/23_containers/vector/ext_pointer/types/1.cc
	* testsuite/23_containers/vector/ext_pointer/types/2.cc
	(N::operator-): Make less specialized.
---
 gcc/cp/pt.cc                                  | 68 ++++++++++++++-----
 .../g++.dg/template/partial-order4.C          | 17 +++++
 gcc/testsuite/g++.dg/template/spec26.C        | 10 +--
 .../vector/ext_pointer/types/1.cc             |  4 +-
 .../vector/ext_pointer/types/2.cc             |  4 +-
 5 files changed, 78 insertions(+), 25 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/partial-order4.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 924a20973b4..4b2af4f7aca 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -25218,27 +25218,61 @@ more_specialized_fn (tree pat1, tree pat2, int len)
   bool lose1 = false;
   bool lose2 = false;
 
-  /* Remove the this parameter from non-static member functions.  If
-     one is a non-static member function and the other is not a static
-     member function, remove the first parameter from that function
-     also.  This situation occurs for operator functions where we
-     locate both a member function (with this pointer) and non-member
-     operator (with explicit first operand).  */
-  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1))
+  /* C++17 [temp.func.order]/3 (CWG532)
+
+     If only one of the function templates M is a non-static member of some
+     class A, M is considered to have a new first parameter inserted in its
+     function parameter list. Given cv as the cv-qualifiers of M (if any), the
+     new parameter is of type "rvalue reference to cv A" if the optional
+     ref-qualifier of M is && or if M has no ref-qualifier and the first
+     parameter of the other template has rvalue reference type. Otherwise, the
+     new parameter is of type "lvalue reference to cv A".  */
+
+  if (DECL_STATIC_FUNCTION_P (decl1) || DECL_STATIC_FUNCTION_P (decl2))
     {
-      len--; /* LEN is the number of significant arguments for DECL1 */
-      args1 = TREE_CHAIN (args1);
-      if (!DECL_STATIC_FUNCTION_P (decl2))
-	args2 = TREE_CHAIN (args2);
-    }
-  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
-    {
-      args2 = TREE_CHAIN (args2);
-      if (!DECL_STATIC_FUNCTION_P (decl1))
+      /* Note C++20 DR2445 extended the above to static member functions, but
+	 I think think the old G++ behavior of just skipping the object
+	 parameter when comparing to a static member function was better, so
+	 let's stick with that for now.  This is CWG2834.  --jason 2023-12 */
+      if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1)) /* FIXME or explicit */
 	{
-	  len--;
+	  len--; /* LEN is the number of significant arguments for DECL1 */
 	  args1 = TREE_CHAIN (args1);
 	}
+      else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2)) /* FIXME or explicit */
+	args2 = TREE_CHAIN (args2);
+    }
+  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) /* FIXME implicit only */
+	   && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
+    {
+      /* Note DR2445 also (IMO wrongly) removed the "only one" above, which
+	 would break e.g.  cpp1y/lambda-generic-variadic5.C.  */
+      len--;
+      args1 = TREE_CHAIN (args1);
+      args2 = TREE_CHAIN (args2);
+    }
+  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) /* FIXME implicit only */
+	   || DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
+    {
+      /* The other is a non-member or explicit object member function;
+	 rewrite the implicit object parameter to a reference.  */
+      tree ns = DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2) ? decl2 : decl1;
+      tree &nsargs = ns == decl2 ? args2 : args1;
+      tree obtype = TREE_TYPE (TREE_VALUE (nsargs));
+
+      nsargs = TREE_CHAIN (nsargs);
+
+      cp_ref_qualifier rqual = type_memfn_rqual (TREE_TYPE (ns));
+      if (rqual == REF_QUAL_NONE)
+	{
+	  tree otherfirst = ns == decl1 ? args2 : args1;
+	  otherfirst = TREE_VALUE (otherfirst);
+	  if (TREE_CODE (otherfirst) == REFERENCE_TYPE
+	      && TYPE_REF_IS_RVALUE (otherfirst))
+	    rqual = REF_QUAL_RVALUE;
+	}
+      obtype = cp_build_reference_type (obtype, rqual == REF_QUAL_RVALUE);
+      nsargs = tree_cons (NULL_TREE, obtype, nsargs);
     }
 
   /* If only one is a conversion operator, they are unordered.  */
diff --git a/gcc/testsuite/g++.dg/template/partial-order4.C b/gcc/testsuite/g++.dg/template/partial-order4.C
new file mode 100644
index 00000000000..89555ab7060
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/partial-order4.C
@@ -0,0 +1,17 @@
+// DR 532
+// PR c++/53499
+// [temp.func.order] says that we do ordering on the first parameter.
+
+struct A
+{
+  template <class T>
+  bool operator==(T);
+};
+
+template <class T, class U>
+bool operator==(T, U);
+
+int main()
+{
+  A() == A();
+}
diff --git a/gcc/testsuite/g++.dg/template/spec26.C b/gcc/testsuite/g++.dg/template/spec26.C
index fad8e3e1519..253d4211153 100644
--- a/gcc/testsuite/g++.dg/template/spec26.C
+++ b/gcc/testsuite/g++.dg/template/spec26.C
@@ -1,13 +1,15 @@
-// { dg-do run }
+// { dg-do compile { target c++11 } }
 // Copyright (C) 2005 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 16 Sep 2005 <nathan@codesourcery.com>
 
 // PR 23519  template specialization ordering (DR214)
 // Origin:  Maxim Yegorushkin <maxim.yegorushkin@gmail.com>
 
+// DR532 clarified that the * expression is ambiguous.
+
 struct A
 {
-    template<class T> int operator+(T&) { return 1;}
+  template<class T> int operator+(T&) = delete;
 };
 
 template<class T> struct B
@@ -16,7 +18,7 @@ template<class T> struct B
   template<typename R> int operator*(R&) {return 3;}
 };
 
-template <typename T, typename R> int operator-(B<T>, R&) {return 4;}
+template <typename T, typename R> int operator-(B<T>, R&) = delete;
 template<class T> int operator+(A&, B<T>&) { return 5;}
 template <typename T> int operator*(T &, A&){return 6;}
 
@@ -30,6 +32,6 @@ int main()
   if ((b - a) != 2)
     return 2;
   
-  if ((b * a) != 6)
+  if ((b * a) != 6)		// { dg-error "ambiguous" }
     return 3;
 }
diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
index 5b3c673898e..2819851076c 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
@@ -36,8 +36,8 @@ namespace N
     X operator+(T, std::size_t)
     { return X(); }
 
-  template<typename T>
-    X operator-(T, T)
+  template<typename T, typename U>
+    X operator-(T, U)
     { return X(); }
 }
 
diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
index cc9519ee618..6c00d1568ae 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
@@ -38,8 +38,8 @@ namespace N
     X operator+(T, std::size_t)
     { return X(); }
 
-  template<typename T>
-    X operator-(T, T)
+  template<typename T, typename U>
+    X operator-(T, U)
     { return X(); }
 }
 

base-commit: 0e7fee57c00ae17611651e0b057dc03b6e276b82
-- 
2.39.3

Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[waffl3x]

Does CWG2834 effect this weird edge case? I couldn't quite grasp the
standardese so I'm not really sure. These are a few cases from a test
that I finalized last night. I ran this by jwakely and he agreed that
the behavior as shown is correct by the standard. I'll also add that
this is also the current behavior of my patch.

template<typename T> concept Constrain = true;

inline constexpr int iobj_fn = 5;
inline constexpr int xobj_fn = 10;

struct S {
  int f(Constrain auto) { return iobj_fn; };
  int f(this S&&, auto) { return xobj_fn; };

  int g(auto) { return iobj_fn; };
  int g(this S&&, Constrain auto) { return xobj_fn; };
};
int main() {
  S s{};
  s.f (0)                   == iobj_fn;
  static_cast<S&&>(s).f (0) == iobj_fn;

  s.g (0)                   == iobj_fn;
  static_cast<S&&>(s).g (0) == xobj_fn;
}


On Tuesday, December 5th, 2023 at 7:21 PM, Jason Merrill <jason@redhat.com> wrote:


> 
> 
> Tested x86_64-pc-linux-gnu. Are the library test changes OK? A reduced
> example of the issue is at https://godbolt.org/z/cPxrcnKjG
> 
> -- 8< --
> 
> Looks like we implemented option 1 (skip the object parameter) for CWG532
> before the issue was resolved, and never updated to the final resolution of
> option 2 (model it as a reference). More recently CWG2445 extended this
> handling to static member functions; I think that's wrong, and have
> opened CWG2834 to address that and how explicit object member functions
> interact with it.
> 
> The FIXME comments are to guide how the explicit object member function
> support should change the uses of DECL_NONSTATIC_MEMBER_FUNCTION_P.
> 
> The library testsuite changes are to make partial ordering work again
> between the generic operator- in the testcase and
> _Pointer_adapter::operator-.
> 
> DR 532
> PR c++/53499
> 
> gcc/cp/ChangeLog:
> 
> * pt.cc (more_specialized_fn): Fix object parameter handling.
> 
> gcc/testsuite/ChangeLog:
> 
> * g++.dg/template/partial-order4.C: New test.
> * g++.dg/template/spec26.C: Adjust for CWG532.
> 
> libstdc++-v3/ChangeLog:
> 
> * testsuite/23_containers/vector/ext_pointer/types/1.cc
> * testsuite/23_containers/vector/ext_pointer/types/2.cc
> (N::operator-): Make less specialized.
> ---
> gcc/cp/pt.cc | 68 ++++++++++++++-----
> .../g++.dg/template/partial-order4.C | 17 +++++
> gcc/testsuite/g++.dg/template/spec26.C | 10 +--
> .../vector/ext_pointer/types/1.cc | 4 +-
> .../vector/ext_pointer/types/2.cc | 4 +-
> 5 files changed, 78 insertions(+), 25 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/template/partial-order4.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 924a20973b4..4b2af4f7aca 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -25218,27 +25218,61 @@ more_specialized_fn (tree pat1, tree pat2, int len)
> bool lose1 = false;
> bool lose2 = false;
> 
> - /* Remove the this parameter from non-static member functions. If
> - one is a non-static member function and the other is not a static
> - member function, remove the first parameter from that function
> - also. This situation occurs for operator functions where we
> - locate both a member function (with this pointer) and non-member
> - operator (with explicit first operand). /
> - if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1))
> + / C++17 [temp.func.order]/3 (CWG532)
> +
> + If only one of the function templates M is a non-static member of some
> + class A, M is considered to have a new first parameter inserted in its
> + function parameter list. Given cv as the cv-qualifiers of M (if any), the
> + new parameter is of type "rvalue reference to cv A" if the optional
> + ref-qualifier of M is && or if M has no ref-qualifier and the first
> + parameter of the other template has rvalue reference type. Otherwise, the
> + new parameter is of type "lvalue reference to cv A". /
> +
> + if (DECL_STATIC_FUNCTION_P (decl1) || DECL_STATIC_FUNCTION_P (decl2))
> {
> - len--; / LEN is the number of significant arguments for DECL1 /
> - args1 = TREE_CHAIN (args1);
> - if (!DECL_STATIC_FUNCTION_P (decl2))
> - args2 = TREE_CHAIN (args2);
> - }
> - else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> - {
> - args2 = TREE_CHAIN (args2);
> - if (!DECL_STATIC_FUNCTION_P (decl1))
> + / Note C++20 DR2445 extended the above to static member functions, but
> + I think think the old G++ behavior of just skipping the object
> + parameter when comparing to a static member function was better, so
> + let's stick with that for now. This is CWG2834. --jason 2023-12 /
> + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1)) / FIXME or explicit /
> {
> - len--;
> + len--; / LEN is the number of significant arguments for DECL1 /
> args1 = TREE_CHAIN (args1);
> }
> + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2)) / FIXME or explicit /
> + args2 = TREE_CHAIN (args2);
> + }
> + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) / FIXME implicit only /
> + && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> + {
> + / Note DR2445 also (IMO wrongly) removed the "only one" above, which
> + would break e.g. cpp1y/lambda-generic-variadic5.C. /
> + len--;
> + args1 = TREE_CHAIN (args1);
> + args2 = TREE_CHAIN (args2);
> + }
> + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) / FIXME implicit only /
> + || DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> + {
> + / The other is a non-member or explicit object member function;
> + rewrite the implicit object parameter to a reference. /
> + tree ns = DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2) ? decl2 : decl1;
> + tree &nsargs = ns == decl2 ? args2 : args1;
> + tree obtype = TREE_TYPE (TREE_VALUE (nsargs));
> +
> + nsargs = TREE_CHAIN (nsargs);
> +
> + cp_ref_qualifier rqual = type_memfn_rqual (TREE_TYPE (ns));
> + if (rqual == REF_QUAL_NONE)
> + {
> + tree otherfirst = ns == decl1 ? args2 : args1;
> + otherfirst = TREE_VALUE (otherfirst);
> + if (TREE_CODE (otherfirst) == REFERENCE_TYPE
> + && TYPE_REF_IS_RVALUE (otherfirst))
> + rqual = REF_QUAL_RVALUE;
> + }
> + obtype = cp_build_reference_type (obtype, rqual == REF_QUAL_RVALUE);
> + nsargs = tree_cons (NULL_TREE, obtype, nsargs);
> }
> 
> / If only one is a conversion operator, they are unordered. */
> diff --git a/gcc/testsuite/g++.dg/template/partial-order4.C b/gcc/testsuite/g++.dg/template/partial-order4.C
> new file mode 100644
> index 00000000000..89555ab7060
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/partial-order4.C
> @@ -0,0 +1,17 @@
> +// DR 532
> +// PR c++/53499
> +// [temp.func.order] says that we do ordering on the first parameter.
> +
> +struct A
> +{
> + template <class T>
> 
> + bool operator==(T);
> +};
> +
> +template <class T, class U>
> 
> +bool operator==(T, U);
> +
> +int main()
> +{
> + A() == A();
> +}
> diff --git a/gcc/testsuite/g++.dg/template/spec26.C b/gcc/testsuite/g++.dg/template/spec26.C
> index fad8e3e1519..253d4211153 100644
> --- a/gcc/testsuite/g++.dg/template/spec26.C
> +++ b/gcc/testsuite/g++.dg/template/spec26.C
> @@ -1,13 +1,15 @@
> -// { dg-do run }
> +// { dg-do compile { target c++11 } }
> // Copyright (C) 2005 Free Software Foundation, Inc.
> // Contributed by Nathan Sidwell 16 Sep 2005 nathan@codesourcery.com
> 
> 
> // PR 23519 template specialization ordering (DR214)
> // Origin: Maxim Yegorushkin maxim.yegorushkin@gmail.com
> 
> 
> +// DR532 clarified that the * expression is ambiguous.
> +
> struct A
> {
> - template<class T> int operator+(T&) { return 1;}
> 
> + template<class T> int operator+(T&) = delete;
> 
> };
> 
> template<class T> struct B
> 
> @@ -16,7 +18,7 @@ template<class T> struct B
> 
> template<typename R> int operator*(R&) {return 3;}
> 
> };
> 
> -template <typename T, typename R> int operator-(B<T>, R&) {return 4;}
> 
> +template <typename T, typename R> int operator-(B<T>, R&) = delete;
> 
> template<class T> int operator+(A&, B<T>&) { return 5;}
> 
> template <typename T> int operator*(T &, A&){return 6;}
> 
> 
> @@ -30,6 +32,6 @@ int main()
> if ((b - a) != 2)
> return 2;
> 
> - if ((b * a) != 6)
> + if ((b * a) != 6) // { dg-error "ambiguous" }
> return 3;
> }
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> index 5b3c673898e..2819851076c 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> @@ -36,8 +36,8 @@ namespace N
> X operator+(T, std::size_t)
> { return X(); }
> 
> - template<typename T>
> 
> - X operator-(T, T)
> + template<typename T, typename U>
> 
> + X operator-(T, U)
> { return X(); }
> }
> 
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> index cc9519ee618..6c00d1568ae 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> @@ -38,8 +38,8 @@ namespace N
> X operator+(T, std::size_t)
> { return X(); }
> 
> - template<typename T>
> 
> - X operator-(T, T)
> + template<typename T, typename U>
> 
> + X operator-(T, U)
> { return X(); }
> }
> 
> 
> base-commit: 0e7fee57c00ae17611651e0b057dc03b6e276b82
> --
> 2.39.3

Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[Jason Merrill]

On 12/5/23 23:23, waffl3x wrote:
> Does CWG2834 effect this weird edge case?

2834 affects all partial ordering with explicit object member functions; 
currently the working draft says that they get an additional fake object 
parameter, which is clearly wrong.

> I couldn't quite grasp the
> standardese so I'm not really sure. These are a few cases from a test
> that I finalized last night. I ran this by jwakely and he agreed that
> the behavior as shown is correct by the standard. I'll also add that
> this is also the current behavior of my patch.
> 
> template<typename T> concept Constrain = true;
> 
> inline constexpr int iobj_fn = 5;
> inline constexpr int xobj_fn = 10;
> 
> struct S {
>    int f(Constrain auto) { return iobj_fn; };
>    int f(this S&&, auto) { return xobj_fn; };
> 
>    int g(auto) { return iobj_fn; };
>    int g(this S&&, Constrain auto) { return xobj_fn; };
> };
> int main() {
>    S s{};
>    s.f (0)                   == iobj_fn;

Yes, the xobj fn isn't viable because it takes an rvalue ref.

>    static_cast<S&&>(s).f (0) == iobj_fn;

Yes, the functions look the same to partial ordering, so we compare 
constraints and the iobj fn is more constrained.

>    s.g (0)                   == iobj_fn;

Yes, the xobj fn isn't viable.

>    static_cast<S&&>(s).g (0) == xobj_fn;

Yes, the xobj fn is more constrained.

Jason

Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[waffl3x]

On Tuesday, December 5th, 2023 at 9:36 PM, Jason Merrill <jason@redhat.com> wrote:


> 
> 
> On 12/5/23 23:23, waffl3x wrote:
> 
> > Does CWG2834 effect this weird edge case?
> 
> 
> 2834 affects all partial ordering with explicit object member functions;

Both in relation to each other, and to iobj and static member functions?

> currently the working draft says that they get an additional fake object
> parameter, which is clearly wrong.

Yeah, that's really weird. I was under the impression that's how static
member functions worked, I didn't realize it was also how it's
specified for xobj member functions. I still find it weird for static
member functions. I guess I'll have to study template partial ordering,
what it is, how it's specified and whatnot. I think I understand it
intuitively but not at a language law level.

> > I couldn't quite grasp the
> > standardese so I'm not really sure. These are a few cases from a test
> > that I finalized last night. I ran this by jwakely and he agreed that
> > the behavior as shown is correct by the standard. I'll also add that
> > this is also the current behavior of my patch.
> > 
> > template<typename T> concept Constrain = true;
> > 
> > inline constexpr int iobj_fn = 5;
> > inline constexpr int xobj_fn = 10;
> > 
> > struct S {
> > int f(Constrain auto) { return iobj_fn; };
> > int f(this S&&, auto) { return xobj_fn; };
> > 
> > int g(auto) { return iobj_fn; };
> > int g(this S&&, Constrain auto) { return xobj_fn; };
> > };
> > int main() {
> > S s{};
> > s.f (0) == iobj_fn;
> 
> 
> Yes, the xobj fn isn't viable because it takes an rvalue ref.
> 
> > static_cast<S&&>(s).f (0) == iobj_fn;
> 
> 
> Yes, the functions look the same to partial ordering, so we compare
> constraints and the iobj fn is more constrained.
> 
> > s.g (0) == iobj_fn;
> 
> 
> Yes, the xobj fn isn't viable.
> 
> > static_cast<S&&>(s).g (0) == xobj_fn;
> 
> 
> Yes, the xobj fn is more constrained.
> 
> Jason

It's funny to see you effortlessly agree with what took me a number of
hours pondering.

So just to confirm, you're also saying the changes proposed by CWG2834
will not change the behavior of this example?

Alex

Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[Jason Merrill]

On 12/6/23 01:02, waffl3x wrote:
> On Tuesday, December 5th, 2023 at 9:36 PM, Jason Merrill <jason@redhat.com> wrote:
>> On 12/5/23 23:23, waffl3x wrote:
>>
>>> Does CWG2834 effect this weird edge case?
>>
>> 2834 affects all partial ordering with explicit object member functions;
> 
> Both in relation to each other, and to iobj and static member functions?
> 
>> currently the working draft says that they get an additional fake object
>> parameter, which is clearly wrong.
> 
> Yeah, that's really weird. I was under the impression that's how static
> member functions worked, I didn't realize it was also how it's
> specified for xobj member functions. I still find it weird for static
> member functions. I guess I'll have to study template partial ordering,
> what it is, how it's specified and whatnot. I think I understand it
> intuitively but not at a language law level.

Right, adding it to static member functions was a recent change, and IMO 
also wrong.

>>> I couldn't quite grasp the
>>> standardese so I'm not really sure. These are a few cases from a test
>>> that I finalized last night. I ran this by jwakely and he agreed that
>>> the behavior as shown is correct by the standard. I'll also add that
>>> this is also the current behavior of my patch.
>>>
>>> template<typename T> concept Constrain = true;
>>>
>>> inline constexpr int iobj_fn = 5;
>>> inline constexpr int xobj_fn = 10;
>>>
>>> struct S {
>>> int f(Constrain auto) { return iobj_fn; };
>>> int f(this S&&, auto) { return xobj_fn; };
>>>
>>> int g(auto) { return iobj_fn; };
>>> int g(this S&&, Constrain auto) { return xobj_fn; };
>>> };
>>> int main() {
>>> S s{};
>>> s.f (0) == iobj_fn;
>>
>> Yes, the xobj fn isn't viable because it takes an rvalue ref.
>>
>>> static_cast<S&&>(s).f (0) == iobj_fn;
>>
>> Yes, the functions look the same to partial ordering, so we compare
>> constraints and the iobj fn is more constrained.
>>
>>> s.g (0) == iobj_fn;
>>
>> Yes, the xobj fn isn't viable.
>>
>>> static_cast<S&&>(s).g (0) == xobj_fn;
>>
>> Yes, the xobj fn is more constrained.
>>
>> Jason
> 
> It's funny to see you effortlessly agree with what took me a number of
> hours pondering.

Well, I've also been thinking about this area, thus the patch.  :)

> So just to confirm, you're also saying the changes proposed by CWG2834
> will not change the behavior of this example?

I'm saying the changes I'm advocating for CWG2834 (the draft you saw on 
github is not at all final) will establish that behavior.

Jason

Re: [PATCH RFA (libstdc++)] c++: partial ordering of object parameter [PR53499]

[Jonathan Wakely]

On Wed, 6 Dec 2023 at 02:21, Jason Merrill wrote:
>
> Tested x86_64-pc-linux-gnu.  Are the library test changes OK?

Sure, they seem fine.

>  A reduced
> example of the issue is at https://godbolt.org/z/cPxrcnKjG
>
> -- 8< --
>
> Looks like we implemented option 1 (skip the object parameter) for CWG532
> before the issue was resolved, and never updated to the final resolution of
> option 2 (model it as a reference).  More recently CWG2445 extended this
> handling to static member functions; I think that's wrong, and have
> opened CWG2834 to address that and how explicit object member functions
> interact with it.
>
> The FIXME comments are to guide how the explicit object member function
> support should change the uses of DECL_NONSTATIC_MEMBER_FUNCTION_P.
>
> The library testsuite changes are to make partial ordering work again
> between the generic operator- in the testcase and
> _Pointer_adapter::operator-.
>
>         DR 532
>         PR c++/53499
>
> gcc/cp/ChangeLog:
>
>         * pt.cc (more_specialized_fn): Fix object parameter handling.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/template/partial-order4.C: New test.
>         * g++.dg/template/spec26.C: Adjust for CWG532.
>
> libstdc++-v3/ChangeLog:
>
>         * testsuite/23_containers/vector/ext_pointer/types/1.cc
>         * testsuite/23_containers/vector/ext_pointer/types/2.cc
>         (N::operator-): Make less specialized.
> ---
>  gcc/cp/pt.cc                                  | 68 ++++++++++++++-----
>  .../g++.dg/template/partial-order4.C          | 17 +++++
>  gcc/testsuite/g++.dg/template/spec26.C        | 10 +--
>  .../vector/ext_pointer/types/1.cc             |  4 +-
>  .../vector/ext_pointer/types/2.cc             |  4 +-
>  5 files changed, 78 insertions(+), 25 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/template/partial-order4.C
>
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 924a20973b4..4b2af4f7aca 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -25218,27 +25218,61 @@ more_specialized_fn (tree pat1, tree pat2, int len)
>    bool lose1 = false;
>    bool lose2 = false;
>
> -  /* Remove the this parameter from non-static member functions.  If
> -     one is a non-static member function and the other is not a static
> -     member function, remove the first parameter from that function
> -     also.  This situation occurs for operator functions where we
> -     locate both a member function (with this pointer) and non-member
> -     operator (with explicit first operand).  */
> -  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1))
> +  /* C++17 [temp.func.order]/3 (CWG532)
> +
> +     If only one of the function templates M is a non-static member of some
> +     class A, M is considered to have a new first parameter inserted in its
> +     function parameter list. Given cv as the cv-qualifiers of M (if any), the
> +     new parameter is of type "rvalue reference to cv A" if the optional
> +     ref-qualifier of M is && or if M has no ref-qualifier and the first
> +     parameter of the other template has rvalue reference type. Otherwise, the
> +     new parameter is of type "lvalue reference to cv A".  */
> +
> +  if (DECL_STATIC_FUNCTION_P (decl1) || DECL_STATIC_FUNCTION_P (decl2))
>      {
> -      len--; /* LEN is the number of significant arguments for DECL1 */
> -      args1 = TREE_CHAIN (args1);
> -      if (!DECL_STATIC_FUNCTION_P (decl2))
> -       args2 = TREE_CHAIN (args2);
> -    }
> -  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> -    {
> -      args2 = TREE_CHAIN (args2);
> -      if (!DECL_STATIC_FUNCTION_P (decl1))
> +      /* Note C++20 DR2445 extended the above to static member functions, but
> +        I think think the old G++ behavior of just skipping the object
> +        parameter when comparing to a static member function was better, so
> +        let's stick with that for now.  This is CWG2834.  --jason 2023-12 */
> +      if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1)) /* FIXME or explicit */
>         {
> -         len--;
> +         len--; /* LEN is the number of significant arguments for DECL1 */
>           args1 = TREE_CHAIN (args1);
>         }
> +      else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2)) /* FIXME or explicit */
> +       args2 = TREE_CHAIN (args2);
> +    }
> +  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) /* FIXME implicit only */
> +          && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> +    {
> +      /* Note DR2445 also (IMO wrongly) removed the "only one" above, which
> +        would break e.g.  cpp1y/lambda-generic-variadic5.C.  */
> +      len--;
> +      args1 = TREE_CHAIN (args1);
> +      args2 = TREE_CHAIN (args2);
> +    }
> +  else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl1) /* FIXME implicit only */
> +          || DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2))
> +    {
> +      /* The other is a non-member or explicit object member function;
> +        rewrite the implicit object parameter to a reference.  */
> +      tree ns = DECL_NONSTATIC_MEMBER_FUNCTION_P (decl2) ? decl2 : decl1;
> +      tree &nsargs = ns == decl2 ? args2 : args1;
> +      tree obtype = TREE_TYPE (TREE_VALUE (nsargs));
> +
> +      nsargs = TREE_CHAIN (nsargs);
> +
> +      cp_ref_qualifier rqual = type_memfn_rqual (TREE_TYPE (ns));
> +      if (rqual == REF_QUAL_NONE)
> +       {
> +         tree otherfirst = ns == decl1 ? args2 : args1;
> +         otherfirst = TREE_VALUE (otherfirst);
> +         if (TREE_CODE (otherfirst) == REFERENCE_TYPE
> +             && TYPE_REF_IS_RVALUE (otherfirst))
> +           rqual = REF_QUAL_RVALUE;
> +       }
> +      obtype = cp_build_reference_type (obtype, rqual == REF_QUAL_RVALUE);
> +      nsargs = tree_cons (NULL_TREE, obtype, nsargs);
>      }
>
>    /* If only one is a conversion operator, they are unordered.  */
> diff --git a/gcc/testsuite/g++.dg/template/partial-order4.C b/gcc/testsuite/g++.dg/template/partial-order4.C
> new file mode 100644
> index 00000000000..89555ab7060
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/partial-order4.C
> @@ -0,0 +1,17 @@
> +// DR 532
> +// PR c++/53499
> +// [temp.func.order] says that we do ordering on the first parameter.
> +
> +struct A
> +{
> +  template <class T>
> +  bool operator==(T);
> +};
> +
> +template <class T, class U>
> +bool operator==(T, U);
> +
> +int main()
> +{
> +  A() == A();
> +}
> diff --git a/gcc/testsuite/g++.dg/template/spec26.C b/gcc/testsuite/g++.dg/template/spec26.C
> index fad8e3e1519..253d4211153 100644
> --- a/gcc/testsuite/g++.dg/template/spec26.C
> +++ b/gcc/testsuite/g++.dg/template/spec26.C
> @@ -1,13 +1,15 @@
> -// { dg-do run }
> +// { dg-do compile { target c++11 } }
>  // Copyright (C) 2005 Free Software Foundation, Inc.
>  // Contributed by Nathan Sidwell 16 Sep 2005 <nathan@codesourcery.com>
>
>  // PR 23519  template specialization ordering (DR214)
>  // Origin:  Maxim Yegorushkin <maxim.yegorushkin@gmail.com>
>
> +// DR532 clarified that the * expression is ambiguous.
> +
>  struct A
>  {
> -    template<class T> int operator+(T&) { return 1;}
> +  template<class T> int operator+(T&) = delete;
>  };
>
>  template<class T> struct B
> @@ -16,7 +18,7 @@ template<class T> struct B
>    template<typename R> int operator*(R&) {return 3;}
>  };
>
> -template <typename T, typename R> int operator-(B<T>, R&) {return 4;}
> +template <typename T, typename R> int operator-(B<T>, R&) = delete;
>  template<class T> int operator+(A&, B<T>&) { return 5;}
>  template <typename T> int operator*(T &, A&){return 6;}
>
> @@ -30,6 +32,6 @@ int main()
>    if ((b - a) != 2)
>      return 2;
>
> -  if ((b * a) != 6)
> +  if ((b * a) != 6)            // { dg-error "ambiguous" }
>      return 3;
>  }
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> index 5b3c673898e..2819851076c 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/1.cc
> @@ -36,8 +36,8 @@ namespace N
>      X operator+(T, std::size_t)
>      { return X(); }
>
> -  template<typename T>
> -    X operator-(T, T)
> +  template<typename T, typename U>
> +    X operator-(T, U)
>      { return X(); }
>  }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> index cc9519ee618..6c00d1568ae 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/ext_pointer/types/2.cc
> @@ -38,8 +38,8 @@ namespace N
>      X operator+(T, std::size_t)
>      { return X(); }
>
> -  template<typename T>
> -    X operator-(T, T)
> +  template<typename T, typename U>
> +    X operator-(T, U)
>      { return X(); }
>  }
>
>
> base-commit: 0e7fee57c00ae17611651e0b057dc03b6e276b82
> --
> 2.39.3
>