Skip to content

[libc++][test] Fix zero-length arrays and copy-pasted lambdas in ranges.contains.pass.cpp #79792

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
merged 3 commits into from
Jan 29, 2024

Conversation

StephanTLavavej
Copy link
Member

  • Fix MSVC error C2466: cannot allocate an array of constant size 0
  • Fix MSVC warning C4805: '==': unsafe mix of type 'int' and type 'const bool' in operation
    • AFAICT, these lambdas were copy-pasted, and didn't intend to take and return int here. This part of the test is using vector<bool> for random-access but non-contiguous iterators, and it's checking how many times the projection is invoked, but the projection doesn't need to do anything squirrely, it should otherwise be an identity.
  • Fix typos: "continuous" => "contiguous".

…t bool' in operation

AFAICT, these lambdas were copy-pasted, and didn't intend to take and return int here. This part of the test is using `vector<bool>` for random-access but non-contiguous iterators,
and it's checking how many times the projection is invoked, but the projection doesn't need to do anything squirrely, it should otherwise be an identity.

Also, fix typos: "continuous" => "contiguous".
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner January 29, 2024 07:22
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 29, 2024

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes
  • Fix MSVC error C2466: cannot allocate an array of constant size 0
    • MSVC rejects this non-Standard extension. Previous fixes: #74183
  • Fix MSVC warning C4805: '==': unsafe mix of type 'int' and type 'const bool' in operation
    • AFAICT, these lambdas were copy-pasted, and didn't intend to take and return int here. This part of the test is using vector&lt;bool&gt; for random-access but non-contiguous iterators, and it's checking how many times the projection is invoked, but the projection doesn't need to do anything squirrely, it should otherwise be an identity.
  • Fix typos: "continuous" => "contiguous".

Full diff: https://github.com/llvm/llvm-project/pull/79792.diff

1 Files Affected:

  • (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains.pass.cpp (+9-8)
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains.pass.cpp
index c928698e453013..7b792bb3ac80c6 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains.pass.cpp
@@ -19,6 +19,7 @@
 //     constexpr bool ranges::contains(R&& r, const T& value, Proj proj = {});                 // since C++23
 
 #include <algorithm>
+#include <array>
 #include <cassert>
 #include <list>
 #include <ranges>
@@ -89,8 +90,8 @@ constexpr void test_iterators() {
   }
 
   { // check that an empty range works
-    ValueT a[] = {};
-    auto whole = std::ranges::subrange(Iter(a), Sent(Iter(a)));
+    std::array<ValueT, 0> a = {};
+    auto whole = std::ranges::subrange(Iter(a.data()), Sent(Iter(a.data())));
     {
       bool ret = std::ranges::contains(whole.begin(), whole.end(), 1);
       assert(!ret);
@@ -164,7 +165,7 @@ constexpr bool test() {
     });
   });
 
-  { // count invocations of the projection for continuous iterators
+  { // count invocations of the projection for contiguous iterators
     int a[]              = {1, 9, 0, 13, 25};
     int projection_count = 0;
     {
@@ -215,22 +216,22 @@ constexpr bool test() {
     }
   }
 
-  { // check invocations of the projection for non-continuous iterators
+  { // check invocations of the projection for non-contiguous iterators
     std::vector<bool> whole{false, false, true, false};
     int projection_count = 0;
     {
-      bool ret = std::ranges::contains(whole.begin(), whole.end(), true, [&](int i) {
+      bool ret = std::ranges::contains(whole.begin(), whole.end(), true, [&](bool b) {
         ++projection_count;
-        return i;
+        return b;
       });
       assert(ret);
       assert(projection_count == 3);
       projection_count = 0;
     }
     {
-      bool ret = std::ranges::contains(whole, true, [&](int i) {
+      bool ret = std::ranges::contains(whole, true, [&](bool b) {
         ++projection_count;
-        return i;
+        return b;
       });
       assert(ret);
       assert(projection_count == 3);

Copy link

github-actions bot commented Jan 29, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@philnik777 philnik777 merged commit ef83894 into llvm:main Jan 29, 2024
@StephanTLavavej StephanTLavavej deleted the ranges-contains branch January 29, 2024 11:03
ldionne pushed a commit that referenced this pull request May 28, 2024
* Guard `std::__make_from_tuple_impl` tests with `#ifdef _LIBCPP_VERSION` and `LIBCPP_STATIC_ASSERT`.
* Change `_LIBCPP_CONSTEXPR_SINCE_CXX20` to `TEST_CONSTEXPR_CXX20`.
+ Other functions in `variant.swap/swap.pass.cpp` were already using the proper test macro.
* Mark `what` as `[[maybe_unused]]` when used by `TEST_LIBCPP_REQUIRE`.
  + This updates one occurrence in `libcxx/test/libcxx` for consistency.
* Windows `_putenv_s()` takes 2 arguments, not 3.
  + See MSVC documentation: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/putenv-s-wputenv-s?view=msvc-170
+ POSIX `setenv()` takes `int overwrite`, but Windows `_putenv_s()` always overwrites.
* Avoid non-Standard zero-length arrays.
  + Followup to #74183 and #79792.
* Add `operator++()` to `unsized_it`.
+ The Standard requires this due to [N4981][] [move.iter.requirements]/1 "The template parameter `Iterator` shall
  either meet the *Cpp17InputIterator* requirements ([input.iterators])
  or model `input_iterator` ([iterator.concept.input])."
+ MSVC's STL requires this because it has a strengthened exception
  specification in `move_iterator` that inspects the underlying iterator's
  increment operator.
* `uniform_int_distribution` forbids `int8_t`/`uint8_t`.
  + See [N4981][] [rand.req.genl]/1.5. MSVC's STL enforces this.
+ Note that when changing the distribution's `IntType`, we need to be
  careful to preserve the original value range of `[0, max_input]`.
* fstreams are constructible from `const fs::path::value_type*` on wide systems.
  + See [ifstream.cons], [ofstream.cons], [fstream.cons].
* In `msvc_stdlib_force_include.h`, map `_HAS_CXX23` to `TEST_STD_VER` 23 instead of 99.
+ On 2023-05-23, 7140050
  started recognizing 23 as a distinct value.
* Fix test name typo: `destory_elements.pass.cpp` => `destroy_elements.pass.cpp`

[N4981]: https://wg21.link/N4981
vg0204 pushed a commit to vg0204/llvm-project that referenced this pull request May 29, 2024
* Guard `std::__make_from_tuple_impl` tests with `#ifdef _LIBCPP_VERSION` and `LIBCPP_STATIC_ASSERT`.
* Change `_LIBCPP_CONSTEXPR_SINCE_CXX20` to `TEST_CONSTEXPR_CXX20`.
+ Other functions in `variant.swap/swap.pass.cpp` were already using the proper test macro.
* Mark `what` as `[[maybe_unused]]` when used by `TEST_LIBCPP_REQUIRE`.
  + This updates one occurrence in `libcxx/test/libcxx` for consistency.
* Windows `_putenv_s()` takes 2 arguments, not 3.
  + See MSVC documentation: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/putenv-s-wputenv-s?view=msvc-170
+ POSIX `setenv()` takes `int overwrite`, but Windows `_putenv_s()` always overwrites.
* Avoid non-Standard zero-length arrays.
  + Followup to llvm#74183 and llvm#79792.
* Add `operator++()` to `unsized_it`.
+ The Standard requires this due to [N4981][] [move.iter.requirements]/1 "The template parameter `Iterator` shall
  either meet the *Cpp17InputIterator* requirements ([input.iterators])
  or model `input_iterator` ([iterator.concept.input])."
+ MSVC's STL requires this because it has a strengthened exception
  specification in `move_iterator` that inspects the underlying iterator's
  increment operator.
* `uniform_int_distribution` forbids `int8_t`/`uint8_t`.
  + See [N4981][] [rand.req.genl]/1.5. MSVC's STL enforces this.
+ Note that when changing the distribution's `IntType`, we need to be
  careful to preserve the original value range of `[0, max_input]`.
* fstreams are constructible from `const fs::path::value_type*` on wide systems.
  + See [ifstream.cons], [ofstream.cons], [fstream.cons].
* In `msvc_stdlib_force_include.h`, map `_HAS_CXX23` to `TEST_STD_VER` 23 instead of 99.
+ On 2023-05-23, llvm@7140050
  started recognizing 23 as a distinct value.
* Fix test name typo: `destory_elements.pass.cpp` => `destroy_elements.pass.cpp`

[N4981]: https://wg21.link/N4981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants