-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libcxx] applies changes regarding post-commit feedback to #75259 #76534
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some of the feedback was also relevant to other files, and has been applied there too.
@llvm/pr-subscribers-libcxx Author: Christopher Di Bella (cjdb) ChangesSome of the feedback was also relevant to other files, and has been applied there too. Full diff: https://github.com/llvm/llvm-project/pull/76534.diff 10 Files Affected:
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
index cf089b27c76e02..ebe00c6b8c258d 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
@@ -56,14 +56,14 @@ concept is_in_value_result =
template <class Result, class T>
concept is_dangling_with = std::same_as<Result, std::ranges::fold_left_with_iter_result<std::ranges::dangling, T>>;
-struct Long {
+struct Integer {
int value;
- constexpr Long(int const x) : value(x) {}
+ constexpr Integer(int const x) : value(x) {}
- constexpr Long plus(int const x) const { return Long{value + x}; }
+ constexpr Integer plus(int const x) const { return Integer{value + x}; }
- friend constexpr bool operator==(Long const& x, Long const& y) = default;
+ friend constexpr bool operator==(Integer const& x, Integer const& y) = default;
};
template <std::ranges::input_range R, class T, class F, std::equality_comparable Expected>
@@ -74,6 +74,7 @@ constexpr void check_iterator(R& r, T const& init, F f, Expected const& expected
assert(result.in == r.end());
assert(result.value == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -89,6 +90,7 @@ constexpr void check_iterator(R& r, T const& init, F f, Expected const& expected
std::same_as<Expected> decltype(auto) result = fold_left(r.begin(), r.end(), init, f);
assert(result == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -108,6 +110,7 @@ constexpr void check_lvalue_range(R& r, T const& init, F f, Expected const& expe
assert(result.in == r.end());
assert(result.value == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -122,6 +125,7 @@ constexpr void check_lvalue_range(R& r, T const& init, F f, Expected const& expe
std::same_as<Expected> decltype(auto) result = fold_left(r, init, f);
assert(result == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -141,6 +145,7 @@ constexpr void check_rvalue_range(R& r, T const& init, F f, Expected const& expe
is_dangling_with<Expected> decltype(auto) result = fold_left_with_iter(std::move(r2), init, f);
assert(result.value == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -157,6 +162,7 @@ constexpr void check_rvalue_range(R& r, T const& init, F f, Expected const& expe
std::same_as<Expected> decltype(auto) result = fold_left(std::move(r2), init, f);
assert(result == expected);
}
+
{
auto telemetry = invocable_telemetry();
auto f2 = invocable_with_telemetry(f, telemetry);
@@ -183,7 +189,7 @@ constexpr void empty_range_test_case() {
check(data, -100, std::multiplies(), -100);
check(data | std::views::take_while([](auto) { return false; }), 1.23, std::plus(), 1.23);
- check(data, Long(52), &Long::plus, Long(52));
+ check(data, Integer(52), &Integer::plus, Integer(52));
}
constexpr void common_range_test_case() {
@@ -206,7 +212,7 @@ constexpr void common_range_test_case() {
};
check(data, 0, fib, fibonacci(data.back()));
- check(data, Long(0), &Long::plus, Long(triangular_sum(data)));
+ check(data, Integer(0), &Integer::plus, Integer(triangular_sum(data)));
}
constexpr void non_common_range_test_case() {
@@ -266,6 +272,7 @@ void runtime_only_test_case() {
assert(result.in == data.end());
assert(result.value == expected);
}
+
{
auto input = std::istringstream(raw_data);
auto data = std::views::istream<std::string>(input);
@@ -274,11 +281,13 @@ void runtime_only_test_case() {
assert(result.in == data.end());
assert(result.value == expected);
}
+
{
auto input = std::istringstream(raw_data);
auto data = std::views::istream<std::string>(input);
assert(fold_left(data.begin(), data.end(), init, std::plus()) == expected);
}
+
{
auto input = std::istringstream(raw_data);
auto data = std::views::istream<std::string>(input);
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.compile.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.compile.pass.cpp
index cad96d4c10127f..261dd6e572e507 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.compile.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.compile.pass.cpp
@@ -10,8 +10,21 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
-// Checks that `std::ranges::fold_left_with_iter`'s requirements reject parameters that don't meet
-// the overloads' constraints.
+// template<input_iterator I, sentinel_for<I> S, class T,
+// indirectly-binary-left-foldable<T, I> F>
+// constexpr see below ranges::fold_left_with_iter(I first, S last, T init, F f);
+//
+// template<input_range R, class T, indirectly-binary-left-foldable<T, iterator_t<R>> F>
+// constexpr see below ranges::fold_left_with_iter(R&& r, T init, F f);
+
+// template<input_iterator I, sentinel_for<I> S, class T,
+// indirectly-binary-left-foldable<T, I> F>
+// constexpr see below ranges::fold_left(I first, S last, T init, F f);
+//
+// template<input_range R, class T, indirectly-binary-left-foldable<T, iterator_t<R>> F>
+// constexpr see below ranges::fold_left(R&& r, T init, F f);
+
+// Checks that the algorithm requirements reject parameters that don't meet the overloads' constraints.
#include <algorithm>
#include <concepts>
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_found_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_found_result.pass.cpp
index af23a1b051c89e..ec104df5feb47e 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_found_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_found_result.pass.cpp
@@ -64,6 +64,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_found_result<double> res{10, true};
assert(res.in == 10);
@@ -72,6 +73,8 @@ constexpr bool test() {
assert(res2.in.content == 10);
assert(res2.found == true);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_found_result<MoveOnly> res{MoveOnly{}, false};
assert(res.in.get() == 1);
@@ -82,10 +85,13 @@ constexpr bool test() {
assert(res.in.get() == 0);
assert(!res.found);
}
- auto [in, found] = std::ranges::in_found_result<int>{2, false};
- assert(in == 2);
- assert(!found);
+ // Checks that structured bindings get the correct values.
+ {
+ auto [in, found] = std::ranges::in_found_result<int>{2, false};
+ assert(in == 2);
+ assert(!found);
+ }
return true;
}
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp
index f44b5988d849ad..4395380ed179aa 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp
@@ -48,7 +48,7 @@ static_assert(std::is_convertible_v<const std::ranges::in_fun_result<int, int>&&
static_assert(std::is_move_constructible_v<std::ranges::in_fun_result<MoveOnly, int>>);
static_assert(std::is_move_constructible_v<std::ranges::in_fun_result<int, MoveOnly>>);
-// should not copy constructible with move-only type
+// should not be copy constructible with move-only type
static_assert(!std::is_copy_constructible_v<std::ranges::in_fun_result<MoveOnly, int>>);
static_assert(!std::is_copy_constructible_v<std::ranges::in_fun_result<int, MoveOnly>>);
@@ -64,6 +64,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_fun_result<int, double> res{10, 0.};
assert(res.in == 10);
@@ -72,6 +73,8 @@ constexpr bool test() {
assert(res2.in.content == 10);
assert(res2.fun.content == 0.);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_fun_result<MoveOnly, int> res{MoveOnly{}, 2};
assert(res.in.get() == 1);
@@ -81,6 +84,8 @@ constexpr bool test() {
assert(res2.in.get() == 1);
assert(res2.fun == 2);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [in, fun] = std::ranges::in_fun_result<int, int>{1, 2};
assert(in == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp
index c9ab2d430b5002..0ee09ceb474167 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp
@@ -67,6 +67,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_in_out_result<int, double, float> res{10, 0., 1.f};
assert(res.in1 == 10);
@@ -77,6 +78,8 @@ constexpr bool test() {
assert(res2.in2.content == 0.);
assert(res2.out.content == 1.f);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_in_out_result<MoveOnly, int, int> res1{MoveOnly{}, 0, 0};
assert(res1.in1.get() == 1);
@@ -84,6 +87,8 @@ constexpr bool test() {
assert(res1.in1.get() == 0);
assert(res2.in1.get() == 1);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [in1, in2, out] = std::ranges::in_in_out_result<int, int, int>{1, 2, 3};
assert(in1 == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_in_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_in_result.pass.cpp
index 64d1106117329c..10e6e88327fe18 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_in_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_in_result.pass.cpp
@@ -59,6 +59,7 @@ struct ConstructibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_in_result<int, double> res{10L, 0.};
assert(res.in1 == 10);
@@ -67,12 +68,16 @@ constexpr bool test() {
assert(res2.in1.content == 10);
assert(res2.in2.content == 0.);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_in_result<MoveOnly, int> res{MoveOnly{}, 0};
assert(res.in1.get() == 1);
[[maybe_unused]] auto res2 = static_cast<std::ranges::in_in_result<MoveOnly, int>>(std::move(res));
assert(res.in1.get() == 0);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [in1, in2] = std::ranges::in_in_result<int, int>{1, 2};
assert(in1 == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_out_out_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_out_out_result.pass.cpp
index 682462bc9a5ba1..875080b1900e05 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_out_out_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_out_out_result.pass.cpp
@@ -65,6 +65,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_out_out_result<int, double, float> res{10, 0., 1.f};
assert(res.in == 10);
@@ -75,6 +76,8 @@ constexpr bool test() {
assert(res2.out1.content == 0.);
assert(res2.out2.content == 1.f);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_out_out_result<MoveOnly, int, int> res1{MoveOnly{}, 0, 0};
assert(res1.in.get() == 1);
@@ -82,6 +85,8 @@ constexpr bool test() {
assert(res1.in.get() == 0);
assert(res2.in.get() == 1);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [in, out1, out2] = std::ranges::in_out_out_result<int, int, int>{1, 2, 3};
assert(in == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp
index ca9f4464dee5fb..9cfb5f43955cb0 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp
@@ -59,6 +59,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_out_result<double, int> res{10, 1};
assert(res.in == 10);
@@ -67,6 +68,8 @@ constexpr bool test() {
assert(res2.in.content == 10);
assert(res2.out.content == 1);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_out_result<MoveOnly, int> res{MoveOnly{}, 10};
assert(res.in.get() == 1);
@@ -77,6 +80,8 @@ constexpr bool test() {
assert(res2.in.get() == 1);
assert(res2.out == 10);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [min, max] = std::ranges::in_out_result<int, int>{1, 2};
assert(min == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp
index f4023b89dce703..b7aba0c88ee950 100644
--- a/libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp
@@ -53,7 +53,7 @@ static_assert(
static_assert(std::is_move_constructible_v<std::ranges::in_value_result<MoveOnly, int>>);
static_assert(std::is_move_constructible_v<std::ranges::in_value_result<int, MoveOnly>>);
-// should not copy constructible with move-only type
+// should not be copy constructible with move-only type
static_assert(!std::is_copy_constructible_v<std::ranges::in_value_result<MoveOnly, int>>);
static_assert(!std::is_copy_constructible_v<std::ranges::in_value_result<int, MoveOnly>>);
@@ -71,6 +71,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::in_value_result<int, double> res{10, 0.};
assert(res.in == 10);
@@ -79,6 +80,8 @@ constexpr bool test() {
assert(res2.in.content == 10);
assert(res2.value.content == 0.);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::in_value_result<MoveOnly, int> res{MoveOnly{}, 2};
assert(res.in.get() == 1);
@@ -88,6 +91,8 @@ constexpr bool test() {
assert(res2.in.get() == 1);
assert(res2.value == 2);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [in, value] = std::ranges::in_value_result<int, int>{1, 2};
assert(in == 1);
diff --git a/libcxx/test/std/algorithms/algorithms.results/min_max_result.pass.cpp b/libcxx/test/std/algorithms/algorithms.results/min_max_result.pass.cpp
index 23c22ec9d3924a..bc394926c9548b 100644
--- a/libcxx/test/std/algorithms/algorithms.results/min_max_result.pass.cpp
+++ b/libcxx/test/std/algorithms/algorithms.results/min_max_result.pass.cpp
@@ -60,6 +60,7 @@ struct ConvertibleFrom {
};
constexpr bool test() {
+ // Checks that conversion operations are correct.
{
std::ranges::min_max_result<double> res{10, 1};
assert(res.min == 10);
@@ -68,6 +69,8 @@ constexpr bool test() {
assert(res2.min.content == 10);
assert(res2.max.content == 1);
}
+
+ // Checks that conversions are possible when one of the types is move-only.
{
std::ranges::min_max_result<MoveOnly> res{MoveOnly{}, MoveOnly{}};
assert(res.min.get() == 1);
@@ -78,6 +81,8 @@ constexpr bool test() {
assert(res2.min.get() == 1);
assert(res2.max.get() == 1);
}
+
+ // Checks that structured bindings get the correct values.
{
auto [min, max] = std::ranges::min_max_result<int>{1, 2};
assert(min == 1);
|
var-const
approved these changes
Mar 27, 2024
The failing test is unrelated to the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some of the feedback was also relevant to other files, and has been applied there too.