Hello,

On 09/10/2024 22:39, Patrick Palka wrote:
+#if __glibcxx_string_view >= 202403L
+  // const string & + string_view
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    [[nodiscard]]
+    constexpr inline basic_string<_CharT, _Traits, _Alloc>

Redundant 'inline's

+    operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
+              type_identity_t<basic_string_view<_CharT, _Traits>> __rhs)
+    {
+      typedef basic_string<_CharT, _Traits, _Alloc> _Str;

These typedefs might as well be usings instead

Besides that LGTM!

Thank you for the review, updated patch attached to fix both of these.
(Just for the record, these had been C&P from the corresponding operator+ overloads that deal with const char *.)

Thanks,
--
Giuseppe D'Angelo
From 19c8c67f81661f41d97ca7c48f5feaf7444d6e48 Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
Date: Tue, 30 Jul 2024 20:09:12 +0200
Subject: [PATCH] libstdc++: implement concatenation of strings and
 string_views

This adds support for P2591R5, merged for C++26.

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h: Implement the four operator+
	overloads between basic_string and (types convertible to)
	basic_string_view.
	* include/bits/version.def: Bump the feature-testing macro.
	* include/bits/version.h: Regenerate.
	* testsuite/21_strings/basic_string/operators/char/op_plus_fspath_neg.cc: New test.
	* testsuite/21_strings/basic_string/operators/char/op_plus_string_view.cc: New test.
	* testsuite/21_strings/basic_string/operators/char/op_plus_string_view_compat.cc:
	New test.

Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
 libstdc++-v3/include/bits/basic_string.h      |  48 +++++
 libstdc++-v3/include/bits/version.def         |   5 +
 libstdc++-v3/include/bits/version.h           |   7 +-
 .../operators/char/op_plus_fspath_neg.cc      |  13 ++
 .../operators/char/op_plus_string_view.cc     | 169 ++++++++++++++++++
 .../char/op_plus_string_view_compat.cc        |  63 +++++++
 6 files changed, 304 insertions(+), 1 deletion(-)
 create mode 100644 libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_fspath_neg.cc
 create mode 100644 libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view.cc
 create mode 100644 libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view_compat.cc

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index e9b17ea48b5..1b49b7e58c3 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3747,6 +3747,54 @@ _GLIBCXX_END_NAMESPACE_CXX11
     { return std::move(__lhs.append(1, __rhs)); }
 #endif
 
+#if __glibcxx_string_view >= 202403L
+  // const string & + string_view
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    [[nodiscard]]
+    constexpr basic_string<_CharT, _Traits, _Alloc>
+    operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
+	       type_identity_t<basic_string_view<_CharT, _Traits>> __rhs)
+    {
+      using _Str = basic_string<_CharT, _Traits, _Alloc>;
+      return std::__str_concat<_Str>(__lhs.data(), __lhs.size(),
+				      __rhs.data(), __rhs.size(),
+				      __lhs.get_allocator());
+    }
+
+  // string && + string_view
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    [[nodiscard]]
+    constexpr basic_string<_CharT, _Traits, _Alloc>
+    operator+(basic_string<_CharT, _Traits, _Alloc>&& __lhs,
+	       type_identity_t<basic_string_view<_CharT, _Traits>> __rhs)
+    {
+      return std::move(__lhs.append(__rhs));
+    }
+
+  // string_view + const string &
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    [[nodiscard]]
+    constexpr basic_string<_CharT, _Traits, _Alloc>
+    operator+(type_identity_t<basic_string_view<_CharT, _Traits>> __lhs,
+	       const basic_string<_CharT, _Traits, _Alloc>& __rhs)
+    {
+      using _Str = basic_string<_CharT, _Traits, _Alloc>;
+      return std::__str_concat<_Str>(__lhs.data(), __lhs.size(),
+				      __rhs.data(), __rhs.size(),
+				      __rhs.get_allocator());
+    }
+
+  // string_view + string &&
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    [[nodiscard]]
+    constexpr basic_string<_CharT, _Traits, _Alloc>
+    operator+(type_identity_t<basic_string_view<_CharT, _Traits>> __lhs,
+	       basic_string<_CharT, _Traits, _Alloc>&& __rhs)
+    {
+      return std::move(__rhs.insert(0, __lhs));
+    }
+#endif
+
   // operator ==
   /**
    *  @brief  Test equivalence of two strings.
diff --git a/libstdc++-v3/include/bits/version.def b/libstdc++-v3/include/bits/version.def
index 477651f358a..b88376bae23 100644
--- a/libstdc++-v3/include/bits/version.def
+++ b/libstdc++-v3/include/bits/version.def
@@ -698,6 +698,11 @@ ftms = {
 
 ftms = {
   name = string_view;
+  values = {
+    v = 202403;
+    cxxmin = 26;
+    hosted = yes;
+  };
   values = {
     v = 201803;
     cxxmin = 17;
diff --git a/libstdc++-v3/include/bits/version.h b/libstdc++-v3/include/bits/version.h
index 342ee6b4c7a..ab551f6a719 100644
--- a/libstdc++-v3/include/bits/version.h
+++ b/libstdc++-v3/include/bits/version.h
@@ -776,7 +776,12 @@
 #undef __glibcxx_want_shared_ptr_weak_type
 
 #if !defined(__cpp_lib_string_view)
-# if (__cplusplus >= 201703L) && _GLIBCXX_HOSTED
+# if (__cplusplus >  202302L) && _GLIBCXX_HOSTED
+#  define __glibcxx_string_view 202403L
+#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_string_view)
+#   define __cpp_lib_string_view 202403L
+#  endif
+# elif (__cplusplus >= 201703L) && _GLIBCXX_HOSTED
 #  define __glibcxx_string_view 201803L
 #  if defined(__glibcxx_want_all) || defined(__glibcxx_want_string_view)
 #   define __cpp_lib_string_view 201803L
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_fspath_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_fspath_neg.cc
new file mode 100644
index 00000000000..bf1e09ada1e
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_fspath_neg.cc
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++17 } }
+
+#include <filesystem>
+#include <string>
+
+int main()
+{
+  std::filesystem::path p = "/var/log/";
+  std::string s = "file";
+  // Concatenation of strings and string views (P2591R5)
+  // should not make this possible:
+  p + s; // { dg-error "no match for" "operator+(string,string_view) should not make this possible" }
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view.cc
new file mode 100644
index 00000000000..364d5bd5aba
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view.cc
@@ -0,0 +1,169 @@
+// { dg-do run { target c++26 } }
+
+#include <string>
+#include <type_traits>
+#include <utility>
+
+#include <testsuite_hooks.h>
+
+#if !defined(__cpp_lib_string_view) || __cpp_lib_string_view < 202403L
+#error "__cpp_lib_string_view should have been defined to 202403L or more"
+#endif
+
+static_assert(std::is_same_v<decltype(std::declval<std::string>() + std::declval<std::string_view>()), std::string>);
+static_assert(std::is_same_v<decltype(std::declval<std::string &>() + std::declval<std::string_view>()), std::string>);
+static_assert(std::is_same_v<decltype(std::declval<std::string_view>() + std::declval<std::string>()), std::string>);
+static_assert(std::is_same_v<decltype(std::declval<std::string_view>() + std::declval<std::string &>()), std::string>);
+
+struct convertible_to_string_view1
+{
+  constexpr operator std::string_view() const { return "convertible_to_sv1"; }
+};
+
+struct convertible_to_string_view2
+{
+  constexpr operator std::string_view()       { return "convertible_to_sv2"; }
+};
+
+struct convertible_to_string_view3
+{
+  constexpr operator std::string_view()       { return "convertible_to_sv3 non_const"; }
+  constexpr operator std::string_view() const { return "convertible_to_sv3 const"; }
+};
+
+struct convertible_to_string_view_and_char_star
+{
+  constexpr operator std::string_view() const { return "convertible_to_sv_and_charstar1"; }
+  constexpr operator const char *() const { return "convertible_to_sv_and_charstar2"; }
+};
+
+struct convertible_to_lots
+{
+  constexpr operator std::string_view() const { return "convertible_to_lots1"; }
+  constexpr operator const char *() const { return "convertible_to_lots2"; }
+  constexpr operator std::string() const { return "convertible_to_lots3"; }
+};
+
+using namespace std::literals;
+static_assert( "costa "s + "marbella"sv == "costa marbella"s );
+static_assert( "costa "sv + "marbella"s == "costa marbella"s );
+static_assert( "costa "s + convertible_to_string_view1{} == "costa convertible_to_sv1"s );
+static_assert( "costa "s + convertible_to_string_view2{} == "costa convertible_to_sv2"s );
+static_assert( "costa "s + convertible_to_string_view3{} == "costa convertible_to_sv3 non_const"s );
+static_assert( "costa "s + convertible_to_string_view_and_char_star{} == "costa convertible_to_sv_and_charstar1"s );
+static_assert( "costa "s + convertible_to_lots{} == "costa convertible_to_lots1"s );
+
+void
+test01()
+{
+  std::string str_0("costa ");
+  std::string str_1("marbella");
+
+  std::string tmp;
+
+  std::string_view str_0_view = str_0;
+  std::string_view str_1_view = str_1;
+
+
+  // string + string_view
+  VERIFY( (str_0 + str_1_view) == "costa marbella" );
+  VERIFY( (str_0 + std::as_const(str_1_view)) == "costa marbella" );
+  VERIFY( (str_0 + std::string_view(str_1)) == "costa marbella" );
+  VERIFY( (str_0_view + str_1) == "costa marbella" );
+  VERIFY( (std::as_const(str_0_view) + str_1) == "costa marbella" );
+  VERIFY( (std::string_view(str_0) + str_1) == "costa marbella" );
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + str_1_view) == "costa marbella" );
+  tmp = str_1;
+  VERIFY( (str_0_view + std::move(tmp)) == "costa marbella" );
+
+
+  // convertible to string_view
+  convertible_to_string_view1 conv_string_view1;
+  VERIFY( (str_0 + conv_string_view1) == "costa convertible_to_sv1" );
+  VERIFY( (str_0 + std::as_const(conv_string_view1)) == "costa convertible_to_sv1" );
+  VERIFY( (std::as_const(str_0) + conv_string_view1) == "costa convertible_to_sv1" );
+  VERIFY( (std::as_const(str_0) + std::as_const(conv_string_view1)) == "costa convertible_to_sv1" );
+
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + conv_string_view1) == "costa convertible_to_sv1" );
+  tmp = str_1;
+  VERIFY( (conv_string_view1 + std::move(tmp)) == "convertible_to_sv1marbella" );
+
+  VERIFY( (conv_string_view1 + str_1) == "convertible_to_sv1marbella" );
+  VERIFY( (conv_string_view1 + std::as_const(str_1)) == "convertible_to_sv1marbella" );
+  VERIFY( (std::as_const(conv_string_view1) + str_1) == "convertible_to_sv1marbella" );
+  VERIFY( (std::as_const(conv_string_view1) + std::as_const(str_1)) == "convertible_to_sv1marbella" );
+
+
+  convertible_to_string_view2 conv_string_view2;
+  VERIFY( (str_0 + conv_string_view2) == "costa convertible_to_sv2" );
+  VERIFY( (std::as_const(str_0) + conv_string_view2) == "costa convertible_to_sv2" );
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + conv_string_view2) == "costa convertible_to_sv2" );
+  tmp = str_1;
+  VERIFY( (conv_string_view2 + std::move(tmp)) == "convertible_to_sv2marbella" );
+
+
+  convertible_to_string_view3 conv_string_view3;
+  VERIFY( (str_0 + conv_string_view3) == "costa convertible_to_sv3 non_const" );
+  VERIFY( (str_0 + std::as_const(conv_string_view3)) == "costa convertible_to_sv3 const" );
+  VERIFY( (std::as_const(str_0) + conv_string_view3) == "costa convertible_to_sv3 non_const" );
+  VERIFY( (std::as_const(str_0) + std::as_const(conv_string_view3)) == "costa convertible_to_sv3 const" );
+
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + conv_string_view3) == "costa convertible_to_sv3 non_const" );
+  tmp = str_1;
+  VERIFY( (conv_string_view3 + std::move(tmp)) == "convertible_to_sv3 non_constmarbella" );
+
+  VERIFY( (conv_string_view3 + str_1) == "convertible_to_sv3 non_constmarbella" );
+  VERIFY( (conv_string_view3 + std::as_const(str_1)) == "convertible_to_sv3 non_constmarbella" );
+  VERIFY( (std::as_const(conv_string_view3) + str_1) == "convertible_to_sv3 constmarbella" );
+  VERIFY( (std::as_const(conv_string_view3) + std::as_const(str_1)) == "convertible_to_sv3 constmarbella" );
+
+
+  convertible_to_string_view_and_char_star conv_sv_cs;
+  VERIFY( (str_0 + conv_sv_cs) == "costa convertible_to_sv_and_charstar1" );
+  VERIFY( (conv_sv_cs + str_1) == "convertible_to_sv_and_charstar1marbella" );
+
+
+  convertible_to_lots conv_lots;
+  VERIFY( (str_0 + conv_lots) == "costa convertible_to_lots1" );
+  VERIFY( (conv_lots + str_1) == "convertible_to_lots1marbella" );
+
+
+  // Check that we're not regressing common cases
+  // string + string literal
+  VERIFY( (str_0 + "marbella") == "costa marbella" );
+  VERIFY( ("costa " + str_1) == "costa marbella" );
+
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + "marbella") == "costa marbella" );
+  tmp = str_1;
+  VERIFY( ("costa " + std::move(tmp)) == "costa marbella" );
+
+
+  // string + non-const char *
+  VERIFY( (str_0 + str_1.data()) == "costa marbella" );
+  VERIFY( (str_0.data() + str_1) == "costa marbella" );
+
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + str_1.data()) == "costa marbella" );
+  tmp = str_1;
+  VERIFY( (str_0.data() + std::move(tmp)) == "costa marbella" );
+
+
+  // string + const char *
+  VERIFY( (str_0 + std::as_const(str_1).data()) == "costa marbella" );
+  VERIFY( (std::as_const(str_0).data() + str_1) == "costa marbella" );
+
+  tmp = str_0;
+  VERIFY( (std::move(tmp) + std::as_const(str_1).data()) == "costa marbella" );
+  tmp = str_1;
+  VERIFY( (std::as_const(str_0).data() + std::move(tmp)) == "costa marbella" );
+}
+
+int main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view_compat.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view_compat.cc
new file mode 100644
index 00000000000..d24f9fb2c1d
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operators/char/op_plus_string_view_compat.cc
@@ -0,0 +1,63 @@
+// { dg-do compile { target c++17 } }
+
+#include <string>
+#include <type_traits>
+#include <utility>
+
+#include <testsuite_hooks.h>
+
+// "legacy" string view and operators
+template <typename CharT>
+struct basic_my_string_view
+{
+  std::basic_string_view<CharT> view;
+};
+
+using my_string_view = basic_my_string_view<char>;
+
+template <class T>
+struct my_type_identity { using type = T; };
+template <class T>
+using my_type_identity_t = typename my_type_identity<T>::type;
+
+template <typename CharT, typename T, typename A>
+std::string operator+(const std::basic_string<CharT, T, A> &s, basic_my_string_view<CharT> msv)
+{
+  std::string result = s;
+  result += msv.view;
+  result += " using my_string_view";
+  return result;
+}
+
+template <typename CharT, typename T, typename A>
+std::string operator+(const std::basic_string<CharT, T, A> &s, my_type_identity_t<basic_my_string_view<CharT>> msv)
+{
+  std::string result = s;
+  result += msv.view;
+  result += " using my_string_view";
+  return result;
+}
+
+
+struct buffer
+{
+  std::string buf;
+
+  // "legacy"
+  operator my_string_view() const { return my_string_view{buf}; }
+  // "modern"
+  operator std::string_view() const { return std::string_view{buf}; }
+};
+
+int
+main()
+{
+  std::string s = "costa ";
+  buffer b{"marbella"};
+
+  // This should be ambiguous in C++26 due to new operator+ overloads
+  // between std::string and objects convertible to std::string_view
+  // (P2591R5)
+  std::string result = s + b; // { dg-error "ambiguous" "operator+(string,string_view) should make this ambiguous" { target c++26 } }
+  VERIFY( result == "costa marbella using my_string_view" );
+}
-- 
2.34.1

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to