On Sat, 21 Sep 2024, Giuseppe D'Angelo wrote:

> On 31/07/2024 00:19, Jonathan Wakely wrote:
> > One more thing that I missed last time, sorry:
> > 
> > +#if __glibcxx_string_view >= 202403L
> > +  // const string & + string_view
> > +  template<typename _CharT, typename _Traits, typename _Alloc>
> > +    _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > +    inline basic_string<_CharT, _Traits, _Alloc>
> > +    operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> > +           __type_identity_t<basic_string_view<_CharT, _Traits>> __rhs)
> > 
> > Since this is a C++26 feature, we can use [[nodiscard]] and constexpr
> > unconditionally, so it can be simply:
> > 
> > +  template<typename _CharT, typename _Traits, typename _Alloc>
> > +    [[nodiscard]]
> > +    constexpr basic_string<_CharT, _Traits, _Alloc>
> > 
> > i.e. use [[nodiscard]] not the NODISCARD macro, and constexpr instead
> > of the CONSTEXPR macro and the inline keyword.
> 
> Here's a rebased patch that also includes these changes.
> 
> Thank you,
> 
> -- 
> Giuseppe D'Angelo
> 

> 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 120c0bc9a17..7cb2193230c 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3745,6 +3745,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 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!

> +      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 inline 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 inline basic_string<_CharT, _Traits, _Alloc>
> +    operator+(type_identity_t<basic_string_view<_CharT, _Traits>> __lhs,
> +            const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> +    {
> +      typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> +      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 inline 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 23478523e0a..d1505c1a0ac 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 0fac749d91e..063ce695bef 100644
> --- a/libstdc++-v3/include/bits/version.h
> +++ b/libstdc++-v3/include/bits/version.h
> @@ -774,7 +774,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

Reply via email to