libstdc++: Do not use CTAD for _Utf32_view alias template

Message ID 20240116212646.1717827-1-jwakely@redhat.com
State Committed
Commit 665a3ff1539ce24e1215e52a14450ecd9a26e87f
Headers
Series libstdc++: Do not use CTAD for _Utf32_view alias template |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed

Commit Message

Jonathan Wakely Jan. 16, 2024, 9:21 p.m. UTC
  Tested aarch64-linux. I plan to push this to fix an error when using
trunk with Clang.

-- >8 --

We were relying on P1814R0 (CTAD for alias templates) which isn't
supported by Clang. We can just not use CTAD and provide an explicit
template argument list for _Utf32_view.

Ideally we'd define a deduction guide for _Grapheme_cluster_view that
uses views::all_t to properly convert non-views to views, but all_t is
defined in <ranges> and we don't want to include all of that in
<bits/unicode.h>. So just make it require a view for now, which can be
cheaply copied.

Although it's not needed yet, it would also be more correct to
specialize enable_borrowed_range for the views in <bits/unicode.h>.

libstdc++-v3/ChangeLog:

	* include/bits/unicode.h (_Grapheme_cluster_view): Require view.
	Do not use CTAD for _Utf32_view.
	(__format_width, __truncate): Do not use CTAD.
	(enable_borrowed_range<_Utf_view<T, R>>): Define specialization.
	(enable_borrowed_range<_Grapheme_cluster_view<R>>): Likewise.
---
 libstdc++-v3/include/bits/unicode.h | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)
  

Comments

Jonathan Wakely Jan. 17, 2024, 12:12 p.m. UTC | #1
On Tue, 16 Jan 2024 at 21:28, Jonathan Wakely wrote:
>
> Tested aarch64-linux. I plan to push this to fix an error when using
> trunk with Clang.

Pushed.

>
> -- >8 --
>
> We were relying on P1814R0 (CTAD for alias templates) which isn't
> supported by Clang. We can just not use CTAD and provide an explicit
> template argument list for _Utf32_view.
>
> Ideally we'd define a deduction guide for _Grapheme_cluster_view that
> uses views::all_t to properly convert non-views to views, but all_t is
> defined in <ranges> and we don't want to include all of that in
> <bits/unicode.h>. So just make it require a view for now, which can be
> cheaply copied.
>
> Although it's not needed yet, it would also be more correct to
> specialize enable_borrowed_range for the views in <bits/unicode.h>.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/unicode.h (_Grapheme_cluster_view): Require view.
>         Do not use CTAD for _Utf32_view.
>         (__format_width, __truncate): Do not use CTAD.
>         (enable_borrowed_range<_Utf_view<T, R>>): Define specialization.
>         (enable_borrowed_range<_Grapheme_cluster_view<R>>): Likewise.
> ---
>  libstdc++-v3/include/bits/unicode.h | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/unicode.h b/libstdc++-v3/include/bits/unicode.h
> index f1b2b359bdf..d35c83d0090 100644
> --- a/libstdc++-v3/include/bits/unicode.h
> +++ b/libstdc++-v3/include/bits/unicode.h
> @@ -714,15 +714,15 @@ inline namespace __v15_1_0
>    };
>
>    // Split a range into extended grapheme clusters.
> -  template<ranges::forward_range _View>
> +  template<ranges::forward_range _View> requires ranges::view<_View>
>      class _Grapheme_cluster_view
>      : public ranges::view_interface<_Grapheme_cluster_view<_View>>
>      {
>      public:
>
>        constexpr
> -      _Grapheme_cluster_view(const _View& __v)
> -      : _M_begin(_Utf32_view(__v).begin())
> +      _Grapheme_cluster_view(_View __v)
> +      : _M_begin(_Utf32_view<_View>(std::move(__v)).begin())
>        { }
>
>        constexpr auto begin() const { return _M_begin; }
> @@ -946,7 +946,7 @@ inline namespace __v15_1_0
>      {
>        if (__s.empty()) [[unlikely]]
>         return 0;
> -      _Grapheme_cluster_view __gc(__s);
> +      _Grapheme_cluster_view<basic_string_view<_CharT>> __gc(__s);
>        auto __it = __gc.begin();
>        const auto __end = __gc.end();
>        size_t __n = __it.width();
> @@ -964,7 +964,7 @@ inline namespace __v15_1_0
>        if (__s.empty()) [[unlikely]]
>         return 0;
>
> -      _Grapheme_cluster_view __gc(__s);
> +      _Grapheme_cluster_view<basic_string_view<_CharT>> __gc(__s);
>        auto __it = __gc.begin();
>        const auto __end = __gc.end();
>        size_t __n = __it.width();
> @@ -1058,6 +1058,19 @@ inline namespace __v15_1_0
>
>  } // namespace __unicode
>
> +namespace ranges
> +{
> +  template<typename _To, typename _Range>
> +    inline constexpr bool
> +    enable_borrowed_range<std::__unicode::_Utf_view<_To, _Range>>
> +      = enable_borrowed_range<_Range>;
> +
> +  template<typename _Range>
> +    inline constexpr bool
> +    enable_borrowed_range<std::__unicode::_Grapheme_cluster_view<_Range>>
> +      = enable_borrowed_range<_Range>;
> +} // namespace ranges
> +
>  _GLIBCXX_END_NAMESPACE_VERSION
>  } // namespace std
>  #endif // C++20
> --
> 2.43.0
>
  

Patch

diff --git a/libstdc++-v3/include/bits/unicode.h b/libstdc++-v3/include/bits/unicode.h
index f1b2b359bdf..d35c83d0090 100644
--- a/libstdc++-v3/include/bits/unicode.h
+++ b/libstdc++-v3/include/bits/unicode.h
@@ -714,15 +714,15 @@  inline namespace __v15_1_0
   };
 
   // Split a range into extended grapheme clusters.
-  template<ranges::forward_range _View>
+  template<ranges::forward_range _View> requires ranges::view<_View>
     class _Grapheme_cluster_view
     : public ranges::view_interface<_Grapheme_cluster_view<_View>>
     {
     public:
 
       constexpr
-      _Grapheme_cluster_view(const _View& __v)
-      : _M_begin(_Utf32_view(__v).begin())
+      _Grapheme_cluster_view(_View __v)
+      : _M_begin(_Utf32_view<_View>(std::move(__v)).begin())
       { }
 
       constexpr auto begin() const { return _M_begin; }
@@ -946,7 +946,7 @@  inline namespace __v15_1_0
     {
       if (__s.empty()) [[unlikely]]
 	return 0;
-      _Grapheme_cluster_view __gc(__s);
+      _Grapheme_cluster_view<basic_string_view<_CharT>> __gc(__s);
       auto __it = __gc.begin();
       const auto __end = __gc.end();
       size_t __n = __it.width();
@@ -964,7 +964,7 @@  inline namespace __v15_1_0
       if (__s.empty()) [[unlikely]]
 	return 0;
 
-      _Grapheme_cluster_view __gc(__s);
+      _Grapheme_cluster_view<basic_string_view<_CharT>> __gc(__s);
       auto __it = __gc.begin();
       const auto __end = __gc.end();
       size_t __n = __it.width();
@@ -1058,6 +1058,19 @@  inline namespace __v15_1_0
 
 } // namespace __unicode
 
+namespace ranges
+{
+  template<typename _To, typename _Range>
+    inline constexpr bool
+    enable_borrowed_range<std::__unicode::_Utf_view<_To, _Range>>
+      = enable_borrowed_range<_Range>;
+
+  template<typename _Range>
+    inline constexpr bool
+    enable_borrowed_range<std::__unicode::_Grapheme_cluster_view<_Range>>
+      = enable_borrowed_range<_Range>;
+} // namespace ranges
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 #endif // C++20