[Bug libstdc++/114400] New: The resolution of LWG3950 seems incorrectly implemented

[Bug libstdc++/114400] New: The resolution of LWG3950 seems incorrectly implemented

[de34 at live dot cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

            Bug ID: 114400
           Summary: The resolution of LWG3950 seems incorrectly
                    implemented
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: de34 at live dot cn
  Target Milestone: ---

The following example fails to compile with libstdc++ due to ambiguity in
overload resolution (https://godbolt.org/z/Ya5o4rrKj).


```
#include <string>
#include <string_view>
#include <type_traits>

namespace test {
    template<class Traits>
    concept characterized_traits = requires { typename
Traits::is_characterized; };

    template<class CharT>
    struct test_traits : std::char_traits<CharT> {
        using is_characterized = void;
    };

    template<class Traits>
    struct traits_comparison_category {
        using type = std::weak_ordering;
    };
    template<class Traits>
        requires requires { typename Traits::comparison_category; }
    struct traits_comparison_category<Traits> {
        using type = Traits::comparison_category;
        static_assert(std::disjunction_v<
            std::is_same<type, std::partial_ordering>,
            std::is_same<type, std::weak_ordering>,
            std::is_same<type, std::strong_ordering>>);
    };

    // N.B. std::type_identity_t is exactly used.

    template<class CharT, characterized_traits Traits>
    constexpr bool operator==(
        std::basic_string_view<CharT, Traits> x,
        std::type_identity_t<std::basic_string_view<CharT, Traits>> y) noexcept
    {
        return x.size() == y.size() && x.compare(y) == 0;
    }

    template<class CharT, characterized_traits Traits>
    constexpr traits_comparison_category<Traits>::type operator<=>(
        std::basic_string_view<CharT, Traits> x,
        std::type_identity_t<std::basic_string_view<CharT, Traits>> y) noexcept
    {
        return
static_cast<traits_comparison_category<Traits>::type>(x.compare(y) <=> 0);
    }

    using test_string_view = std::basic_string_view<char, test_traits<char>>;
    static_assert(test_string_view{} == test_string_view{});
    static_assert(test_string_view{} == "");
    static_assert("" == test_string_view{});

    static_assert(test_string_view{} <=> test_string_view{} ==
std::strong_ordering::equal);
    static_assert(test_string_view{} <=> "" == std::strong_ordering::equal);
    static_assert("" <=> test_string_view{} == std::strong_ordering::equal);
}
```

The resolution of LWG3950 (https://cplusplus.github.io/LWG/issue3950) uses
`type_identity_t`, while libstdc++ currently uses `__type_identity_t`. The
difference between two alias templates can be observed by partial ordering
introduced by associated constraints.

Related PR in MSVC STL:
https://github.com/microsoft/STL/pull/4249

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The resolution of LWG 3950 has not been implemented, at all. The use of
__type_identity_t there dates from 2019 (r10-1886-g0d67cd380d37f2) and replaced
earlier uses of common_type_t which date from the initial commit of
std::experimental::string_view (r0-126555-g77cba5af77ccf8).

The current code is 100% conforming, since the method of disambiguation is
unspecified currently.  LWG 3950 should be approved in a few days, and then
we'll have to decide what to do (we can't use type_identity unconditionally
because it doesn't exist in C++17, which is why I used __type_identity_t in the
first place).

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
One option would be to make __type_identity_t an alias for type_identity_t in
C++20:

--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -156,13 +156,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     using __conditional_t
       = typename __conditional<_Cond>::template type<_If, _Else>;

+#ifdef __cpp_lib_type_identity // C++ >= 20
+  /** * Identity metafunction.
+   * @since C++20
+   * @{
+   */
+  template<typename _Tp>
+    struct type_identity { using type = _Tp; };
+
+  template<typename _Tp>
+    using type_identity_t = typename type_identity<_Tp>::type;
+  /// @}
+
+  template<typename _Tp>
+    using __type_identity_t = type_identity_t<_Tp>;
+  template<typename _Tp>
+    using __type_identity_t = type_identity_t<_Tp>;
+#else
   /// @cond undocumented
   template <typename _Type>
-    struct __type_identity
-    { using type = _Type; };
+    using __type_identity = type_identity<_Type>;

   template<typename _Tp>
     using __type_identity_t = typename __type_identity<_Tp>::type;
+#endif

   namespace __detail
   {
@@ -3600,19 +3617,6 @@ template<typename _Ret, typename _Fn, typename... _Args>
   /// @}
 #endif // __cpp_lib_remove_cvref

-#ifdef __cpp_lib_type_identity // C++ >= 20
-  /** * Identity metafunction.
-   * @since C++20
-   * @{
-   */
-  template<typename _Tp>
-    struct type_identity { using type = _Tp; };
-
-  template<typename _Tp>
-    using type_identity_t = typename type_identity<_Tp>::type;
-  /// @}
-#endif
-
 #ifdef __cpp_lib_unwrap_ref // C++ >= 20
   /** Unwrap a reference_wrapper
    * @since C++20

I don't think we have any uses of __type_identity or __type_identity_t where
this will cause ABI changes. None of the types using __type_identity_t in
constructors are explicitly instantiated in the library.

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Oops, that was meant to be:

#ifdef __cpp_lib_type_identity // C++ >= 20
  /** * Identity metafunction.
   * @since C++20
   * @{
   */
  template<typename _Tp>
    struct type_identity { using type = _Tp; };

  template<typename _Tp>
    using type_identity_t = typename type_identity<_Tp>::type;
  /// @}

  template<typename _Tp>
    using __type_identity = type_identity<_Tp>;
  template<typename _Tp>
    using __type_identity_t = type_identity_t<_Tp>;
#else
  /// @cond undocumented
  template <typename _Type>
    using __type_identity
    { using type = _Type; };

  template<typename _Tp>
    using __type_identity_t = typename __type_identity<_Tp>::type;
#endif

But that messes up the @cond / @endcond nesting. Anyway, something like that.

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[de34 at live dot cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #4 from Jiang An <de34 at live dot cn> ---
(In reply to Jonathan Wakely from comment #1)
> The resolution of LWG 3950 has not been implemented, at all.

Hmm... r14-5349 seems "implementing the resolution" per the commit message.
Perhaps I misread something.

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It was simplifying the overload set, consistent with the proposed resolution of
3950.

If 3950 breaks existing implementations, then arguably 3950 is wrong and should
not be approved.

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Alternatively:

--- a/libstdc++-v3/include/std/string_view
+++ b/libstdc++-v3/include/std/string_view
@@ -602,15 +602,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // deduction and the other argument gets implicitly converted to the deduced
   // type (see N3766).

+#if __cpp_lib_three_way_comparison
   template<typename _CharT, typename _Traits>
     [[nodiscard]]
     constexpr bool
     operator==(basic_string_view<_CharT, _Traits> __x,
-               __type_identity_t<basic_string_view<_CharT, _Traits>> __y)
+              type_identity_t<basic_string_view<_CharT, _Traits>> __y)
     noexcept
     { return __x.size() == __y.size() && __x.compare(__y) == 0; }

-#if __cpp_lib_three_way_comparison
   template<typename _CharT, typename _Traits>
     [[nodiscard]]
     constexpr auto
@@ -620,7 +620,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     -> decltype(__detail::__char_traits_cmp_cat<_Traits>(0))
     { return __detail::__char_traits_cmp_cat<_Traits>(__x.compare(__y)); }
 #else
-  template<typename _CharT, typename _Traits>
+  template<typena  template<typename _CharT, typename _Traits>
+    [[nodiscard]]
+    constexpr bool
+    operator==(basic_string_view<_CharT, _Traits> __x,
+              __type_identity_t<basic_string_view<_CharT, _Traits>> __y)
+    noexcept
+    { return __x.size() == __y.size() && __x.compare(__y) == 0; }
+
+me _CharT, typename _Traits>
     [[nodiscard]]
     constexpr bool
     operator==(basic_string_view<_CharT, _Traits> __x,


Our other uses of __type_identity_t (e.g. in container constructors) are
probably OK. Users can't overload those, and the exact signatures of member
functions are unspecified anyway (we can add/remove default arguments).

[Bug libstdc++/114400] The resolution of LWG3950 seems incorrectly implemented

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114400

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:f4605c53ea2eeafc13e14dd1ad00a0caf80057e2

commit r14-9642-gf4605c53ea2eeafc13e14dd1ad00a0caf80057e2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Mar 20 11:07:56 2024 +0000

    libstdc++: Use std::type_identity_t in <string_view> as per LWG 3950
[PR114400]

    The difference between __type_identity_t and std::type_identity_t is
    observable, as demonstrated in the PR. Nobody in LWG seems to think this
    an example we should really care about, but it seems easy and harmless
    to change this.

    libstdc++-v3/ChangeLog:

            PR libstdc++/114400
            * include/std/string_view (operator==): Use std::type_identity_t
            in C++20 instead of our own __type_identity_t.