Skip to content

Commit a553bc5

Browse files
H-G-HristovH-G-Hristov
committed
Adressed review comments
1 parent bbaf7ba commit a553bc5

File tree

6 files changed

+107
-43
lines changed

6 files changed

+107
-43
lines changed

‎libcxx/include/string‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1766,7 +1766,7 @@ public:
17661766
# endif
17671767

17681768
# if _LIBCPP_STD_VER >= 26
1769-
_LIBCPP_HIDE_FROM_ABI constexpr basic_string_view<_CharT, _Traits>
1769+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr basic_string_view<_CharT, _Traits>
17701770
subview(size_type __pos = 0, size_type __n = npos) const {
17711771
return basic_string_view<_CharT, _Traits>(*this).subview(__pos, __n);
17721772
}

‎libcxx/include/string_view‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -468,12 +468,9 @@ public:
468468
}
469469

470470
# if _LIBCPP_STD_VER >= 26
471-
_LIBCPP_HIDE_FROM_ABI constexpr basic_string_view subview(size_type __pos = 0, size_type __n = npos) const {
472-
// Use the `__assume_valid` form of the constructor to avoid an unnecessary check. Any subview of a view is a
473-
// valid view. In particular, `size()` is known to be smaller than `numeric_limits<difference_type>::max()`, so the
474-
// new size is also smaller.
475-
return __pos > size() ? (__throw_out_of_range("string_view::subview"), basic_string_view())
476-
: basic_string_view(__assume_valid(), data() + __pos, std::min(__n, size() - __pos));
471+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr basic_string_view
472+
subview(size_type __pos = 0, size_type __n = npos) const {
473+
return this->substr(__pos, __n);
477474
}
478475
# endif
479476

‎libcxx/test/std/strings/basic.string/string.ops/string_substr/nodiscard.verify.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// #include <allocator>
10+
#include <string>
11+
12+
#include "constexpr_char_traits.h"
13+
#include "test_macros.h"
14+
#include "type_algorithms.h"
15+
16+
template <typename CharT, typename TraitsT, typename AllocT>
17+
constexpr void test() {
18+
#if TEST_STD_VER >= 26
19+
std::basic_string<CharT, TraitsT, AllocT> s;
20+
21+
s.subview(); // expected-warning {{ignoring return value of function}}
22+
#endif
23+
}
24+
25+
class Test {
26+
public:
27+
template <typename CharT>
28+
constexpr void operator()() const {
29+
test<CharT, std::char_traits<CharT>, std::allocator<CharT>>();
30+
}
31+
};
32+
33+
void test() { types::for_each(types::character_types(), Test{}); }

‎libcxx/test/std/strings/basic.string/string.ops/string_substr/subview.pass.cpp‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "min_allocator.h"
2424
#include "test_allocator.h"
2525
#include "test_macros.h"
26+
#include "type_algorithms.h"
2627

2728
#define CS(S) MAKE_CSTRING(CharT, S)
2829

@@ -61,33 +62,33 @@ constexpr void test() {
6162
{ // With a position that is out of range.
6263
try {
6364
s.subview(s.size() + 1);
64-
assert(false && "Expected std::out_of_range exception");
65+
assert(false);
6566
} catch ([[maybe_unused]] const std::out_of_range& ex) {
66-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
67+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
6768
} catch (...) {
68-
assert(false && "Expected std::out_of_range exception");
69+
assert(false);
6970
}
7071
}
7172

7273
{ // With a position that is out of range and a 0 character length.
7374
try {
7475
s.subview(s.size() + 1, 0);
75-
assert(false && "Expected std::out_of_range exception");
76+
assert(false);
7677
} catch ([[maybe_unused]] const std::out_of_range& ex) {
77-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
78+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
7879
} catch (...) {
79-
assert(false && "Expected std::out_of_range exception");
80+
assert(false);
8081
}
8182
}
8283

8384
{ // With a position that is out of range and a some character length.
8485
try {
8586
s.subview(s.size() + 1, 1);
86-
assert(false && "Expected std::out_of_range exception");
87+
assert(false);
8788
} catch ([[maybe_unused]] const std::out_of_range& ex) {
88-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
89+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
8990
} catch (...) {
90-
assert(false && "Expected std::out_of_range exception");
91+
assert(false);
9192
}
9293
}
9394
}
@@ -107,16 +108,16 @@ constexpr void test() {
107108
test<CharT, constexpr_char_traits<CharT>, test_allocator<CharT>>();
108109
}
109110

111+
class Test {
112+
public:
113+
template <typename CharT>
114+
constexpr void operator()() const {
115+
test<CharT>();
116+
}
117+
};
118+
110119
constexpr bool test() {
111-
test<char>();
112-
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
113-
test<wchar_t>();
114-
#endif
115-
#ifndef TEST_HAS_NO_CHAR8_T
116-
test<char8_t>();
117-
#endif
118-
test<char16_t>();
119-
test<char32_t>();
120+
types::for_each(types::character_types(), Test{});
120121

121122
return true;
122123
}

‎libcxx/test/std/strings/string.view/string.view.ops/nodiscard.verify.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include <string_view>
10+
11+
#include "constexpr_char_traits.h"
12+
#include "test_macros.h"
13+
#include "type_algorithms.h"
14+
15+
template <typename CharT, typename TraitsT, typename AllocT>
16+
constexpr void test() {
17+
#if TEST_STD_VER >= 26
18+
std::basic_string_view<CharT, TraitsT> sv;
19+
20+
sv.subview(); // expected-warning {{ignoring return value of function}}
21+
#endif
22+
}
23+
24+
class Test {
25+
public:
26+
template <typename CharT>
27+
constexpr void operator()() const {
28+
test<CharT, std::char_traits<CharT>>();
29+
}
30+
};
31+
32+
void test() { types::for_each(types::character_types(), Test{}); }

‎libcxx/test/std/strings/string.view/string.view.ops/subview.pass.cpp‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "constexpr_char_traits.h"
2222
#include "make_string.h"
2323
#include "test_macros.h"
24+
#include "type_algorithms.h"
2425

2526
#define CS(S) MAKE_CSTRING(CharT, S)
2627

@@ -59,33 +60,33 @@ constexpr void test() {
5960
{ // With a position that is out of range.
6061
try {
6162
sv.subview(sv.size() + 1);
62-
assert(false && "Expected std::out_of_range exception");
63+
assert(false);
6364
} catch ([[maybe_unused]] const std::out_of_range& ex) {
64-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
65+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
6566
} catch (...) {
66-
assert(false && "Expected std::out_of_range exception");
67+
assert(false);
6768
}
6869
}
6970

7071
{ // With a position that is out of range and a 0 character length.
7172
try {
7273
sv.subview(sv.size() + 1, 0);
73-
assert(false && "Expected std::out_of_range exception");
74+
assert(false);
7475
} catch ([[maybe_unused]] const std::out_of_range& ex) {
75-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
76+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
7677
} catch (...) {
77-
assert(false && "Expected std::out_of_range exception");
78+
assert(false);
7879
}
7980
}
8081

8182
{ // With a position that is out of range and a some character length.
8283
try {
8384
sv.subview(sv.size() + 1, 1);
84-
assert(false && "Expected std::out_of_range exception");
85+
assert(false);
8586
} catch ([[maybe_unused]] const std::out_of_range& ex) {
86-
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::subview");
87+
LIBCPP_ASSERT(std::string(ex.what()) == "string_view::substr");
8788
} catch (...) {
88-
assert(false && "Expected std::out_of_range exception");
89+
assert(false);
8990
}
9091
}
9192
}
@@ -105,16 +106,16 @@ constexpr void test() {
105106
test<CharT, constexpr_char_traits<CharT>>();
106107
}
107108

109+
class Test {
110+
public:
111+
template <typename CharT>
112+
constexpr void operator()() const {
113+
test<CharT>();
114+
}
115+
};
116+
108117
constexpr bool test() {
109-
test<char>();
110-
#ifndef TEST_HAS_NO_WIDE_CHARACTERS
111-
test<wchar_t>();
112-
#endif
113-
#ifndef TEST_HAS_NO_CHAR8_T
114-
test<char8_t>();
115-
#endif
116-
test<char16_t>();
117-
test<char32_t>();
118+
types::for_each(types::character_types(), Test{});
118119

119120
return true;
120121
}

0 commit comments

Comments
 (0)