-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++][strings] P2591R5: Concatenation of strings and string views #88389
base: main
Are you sure you want to change the base?
Changes from all commits
2d606f4
af56ab9
474d5b9
38f6e85
ea270b6
292cd8f
f312052
9a86ea1
695b4b6
2df55a5
6b77e5a
92d08d6
694e6fd
be56164
20b9120
a8c28fb
16fcbba
bdbaf38
5d804f5
6e4d6b4
9f27ece
43c78be
d56da5f
f4f26df
da99993
907bcc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20, c++23 | ||
|
||
// <string> | ||
|
||
// [string.op.plus] | ||
// | ||
// template<class charT, class traits, class Allocator> | ||
// constexpr basic_string<charT, traits, Allocator> | ||
// operator+(const basic_string<charT, traits, Allocator>& lhs, | ||
// type_identity_t<basic_string_view<charT, traits>> rhs); // Since C++26 | ||
// template<class charT, class traits, class Allocator> | ||
// constexpr basic_string<charT, traits, Allocator> | ||
// operator+(basic_string<charT, traits, Allocator>&& lhs, | ||
// type_identity_t<basic_string_view<charT, traits>> rhs); // Since C++26 | ||
// template<class charT, class traits, class Allocator> | ||
// constexpr basic_string<charT, traits, Allocator> | ||
// operator+(type_identity_t<basic_string_view<charT, traits>> lhs, | ||
// const basic_string<charT, traits, Allocator>& rhs); // Since C++26 | ||
// template<class charT, class traits, class Allocator> | ||
// constexpr basic_string<charT, traits, Allocator> | ||
// operator+(type_identity_t<basic_string_view<charT, traits>> lhs, | ||
// basic_string<charT, traits, Allocator>&& rhs); // Since C++26 | ||
Comment on lines
+15
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Splitting the test for each overload will result in too much boilerplate. Should I do it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quite often we do it separately. I see the other operator+ overloads of |
||
|
||
#include <cassert> | ||
#include <concepts> | ||
#include <string> | ||
#include <utility> | ||
|
||
#include "asan_testing.h" | ||
#include "constexpr_char_traits.h" | ||
#include "make_string.h" | ||
#include "min_allocator.h" | ||
#include "test_allocator.h" | ||
#include "test_macros.h" | ||
|
||
#define CS(S) MAKE_CSTRING(CharT, S) | ||
#define ST(S, a) std::basic_string<CharT, TraitsT, AllocT>(MAKE_CSTRING(CharT, S), MKSTR_LEN(CharT, S), a) | ||
#define SV(S) std::basic_string_view<CharT, TraitsT>(MAKE_CSTRING(CharT, S), MKSTR_LEN(CharT, S)) | ||
|
||
template <typename CharT, typename TraitsT, typename AllocT> | ||
constexpr void test(const CharT* x, const CharT* y, const CharT* expected) { | ||
AllocT allocator; | ||
|
||
// string& + string_view | ||
{ | ||
std::basic_string<CharT, TraitsT, AllocT> st{x, allocator}; | ||
std::basic_string_view<CharT, TraitsT> sv{y}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = st + sv; | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(st + sv)); | ||
} | ||
// const string& + string_view | ||
{ | ||
const std::basic_string<CharT, TraitsT, AllocT> st{x, allocator}; | ||
std::basic_string_view<CharT, TraitsT> sv{y}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = st + sv; | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(st + sv)); | ||
} | ||
// string&& + string_view | ||
{ | ||
std::basic_string<CharT, TraitsT, AllocT> st{x, allocator}; | ||
std::basic_string_view<CharT, TraitsT> sv{y}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = std::move(st) + sv; | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(std::move(st) + sv)); | ||
} | ||
// string_view + string& | ||
{ | ||
std::basic_string_view<CharT, TraitsT> sv{x}; | ||
std::basic_string<CharT, TraitsT, AllocT> st{y, allocator}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = sv + st; | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(sv + st)); | ||
} | ||
// string_view + const string& | ||
{ | ||
std::basic_string_view<CharT, TraitsT> sv{x}; | ||
const std::basic_string<CharT, TraitsT, AllocT> st{y, allocator}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = sv + st; | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(sv + st)); | ||
} | ||
// string_view + string&& | ||
{ | ||
// Create a `basic_string` to workaround clang bug: | ||
// https://github.com/llvm/llvm-project/issues/92382 | ||
// Comparison between pointers to a string literal and some other object results in constant evaluation failure. | ||
std::basic_string<CharT, TraitsT, AllocT> st_{x, allocator}; | ||
std::basic_string_view<CharT, TraitsT> sv{st_}; | ||
std::basic_string<CharT, TraitsT, AllocT> st{y, allocator}; | ||
|
||
std::same_as<std::basic_string<CharT, TraitsT, AllocT>> decltype(auto) result = sv + std::move(st); | ||
assert(result == expected); | ||
assert(result.get_allocator() == allocator); | ||
LIBCPP_ASSERT(is_string_asan_correct(sv + std::move(st))); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it also make sense to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure it would! Thank you for suggesting! |
||
} | ||
|
||
template <typename CharT, typename TraitsT, typename AllocT = std::allocator<CharT>> | ||
constexpr void test() { | ||
// Concatenate with an empty `string`/`string_view` | ||
test<CharT, TraitsT, AllocT>(CS(""), CS(""), CS("")); | ||
test<CharT, TraitsT, AllocT>(CS(""), CS("short"), CS("short")); | ||
test<CharT, TraitsT, AllocT>(CS(""), CS("not so short"), CS("not so short")); | ||
test<CharT, TraitsT, AllocT>(CS(""), CS("this is a much longer string"), CS("this is a much longer string")); | ||
|
||
test<CharT, TraitsT, AllocT>(CS(""), CS(""), CS("")); | ||
test<CharT, TraitsT, AllocT>(CS("short"), CS(""), CS("short")); | ||
test<CharT, TraitsT, AllocT>(CS("not so short"), CS(""), CS("not so short")); | ||
test<CharT, TraitsT, AllocT>(CS("this is a much longer string"), CS(""), CS("this is a much longer string")); | ||
|
||
// Non empty | ||
test<CharT, TraitsT, AllocT>(CS("B"), CS("D"), CS("BD")); | ||
test<CharT, TraitsT, AllocT>(CS("zmt94"), CS("+hkt82"), CS("zmt94+hkt82")); | ||
test<CharT, TraitsT, AllocT>(CS("not so short"), CS("+is not bad"), CS("not so short+is not bad")); | ||
test<CharT, TraitsT, AllocT>( | ||
CS("this is a much longer string"), | ||
CS("+which is so much better"), | ||
CS("this is a much longer string+which is so much better")); | ||
} | ||
|
||
template <typename CharT> | ||
constexpr bool test() { | ||
test<CharT, std::char_traits<CharT>>(); | ||
test<CharT, std::char_traits<CharT>, min_allocator<CharT>>(); | ||
test<CharT, std::char_traits<CharT>, safe_allocator<CharT>>(); | ||
test<CharT, std::char_traits<CharT>, test_allocator<CharT>>(); | ||
|
||
test<CharT, constexpr_char_traits<CharT>>(); | ||
test<CharT, constexpr_char_traits<CharT>, min_allocator<CharT>>(); | ||
test<CharT, constexpr_char_traits<CharT>, safe_allocator<CharT>>(); | ||
test<CharT, constexpr_char_traits<CharT>, test_allocator<CharT>>(); | ||
|
||
return true; | ||
} | ||
|
||
int main(int, char**) { | ||
test<char>(); | ||
static_assert(test<char>()); | ||
#ifndef TEST_HAS_NO_WIDE_CHARACTERS | ||
test<wchar_t>(); | ||
static_assert(test<wchar_t>()); | ||
#endif | ||
#ifndef TEST_HAS_NO_CHAR8_T | ||
test<char8_t>(); | ||
static_assert(test<char8_t>()); | ||
#endif | ||
test<char16_t>(); | ||
static_assert(test<char16_t>()); | ||
test<char32_t>(); | ||
static_assert(test<char32_t>()); | ||
|
||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether it would be better to drop
std::move()
here as__lhs
is move-eligible here (ditto below).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review. I reverted my interpretations to the original wording in the paper, so I'll keep this as is for now as I understand it this is our practice and also the older implementations uses the same style.