Skip to content

Conversation

StephanTLavavej
Copy link
Member

Found while running libc++'s test suite with MSVC's STL.

This is structured into a series of commits for easier reviewing; I could also split this into smaller PRs if desired.

  • Add void-casts for invoke_r calls to fix MSVC STL [[nodiscard]] warnings.
    • Our rationale is that if someone is calling invoke_r<NonVoidType>, it sure looks like they care about the return value.
  • Add [[maybe_unused]] to silence -Wunused-parameter warnings.
    • This happens because the parameters are used within LIBCPP_ASSERT, which vanishes for MSVC's STL. This also motivates the following changes.
  • Add [[maybe_unused]] to fix -Wunused-variable warnings.
  • Always void-cast debug_comparisons to fix -Wunused-variable warnings.
    • As this was already unused with a void-cast in one _LIBCPP_HARDENING_MODE branch, I'm simply lifting it next to the variable definition.
  • Add [[maybe_unused]] to fix -Wunused-local-typedef warnings.

@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 26, 2023 10:53
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 26, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2023

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes

Found while running libc++'s test suite with MSVC's STL.

This is structured into a series of commits for easier reviewing; I could also split this into smaller PRs if desired.

  • Add void-casts for invoke_r calls to fix MSVC STL [[nodiscard]] warnings.
    • Our rationale is that if someone is calling invoke_r&lt;NonVoidType&gt;, it sure looks like they care about the return value.
  • Add [[maybe_unused]] to silence -Wunused-parameter warnings.
    • This happens because the parameters are used within LIBCPP_ASSERT, which vanishes for MSVC's STL. This also motivates the following changes.
  • Add [[maybe_unused]] to fix -Wunused-variable warnings.
  • Always void-cast debug_comparisons to fix -Wunused-variable warnings.
    • As this was already unused with a void-cast in one _LIBCPP_HARDENING_MODE branch, I'm simply lifting it next to the variable definition.
  • Add [[maybe_unused]] to fix -Wunused-local-typedef warnings.

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

20 Files Affected:

  • (modified) libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/complexity.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/deque/deque.cons/from_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/deque/deque.modifiers/assign_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/deque/deque.modifiers/insert_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp (+1-1)
  • (modified) libcxx/test/std/strings/basic.string/string.modifiers/string_append/append_range.pass.cpp (+2-2)
  • (modified) libcxx/test/std/strings/basic.string/string.modifiers/string_assign/assign_range.pass.cpp (+2-2)
  • (modified) libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp (+2-2)
  • (modified) libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp (+4-4)
  • (modified) libcxx/test/std/utilities/function.objects/func.invoke/invoke_r.pass.cpp (+2-2)
  • (modified) libcxx/test/support/deduction_guides_sfinae_checks.h (+2-2)
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/complexity.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/complexity.pass.cpp
index dbf7b534069517a..3b7c0ae0b80f08b 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/complexity.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/complexity.pass.cpp
@@ -61,6 +61,7 @@ int main(int, char**) {
     const int debug_elements = std::min(100, n);
     // Multiplier 2 because of comp(a,b) comp(b, a) checks.
     const int debug_comparisons = 2 * (debug_elements + 1) * debug_elements;
+    (void)debug_comparisons;
     std::shuffle(first, last, g);
     std::make_heap(first, last);
     // The exact stats of our current implementation are recorded here.
@@ -70,7 +71,6 @@ int main(int, char**) {
     LIBCPP_ASSERT(stats.moved <= 2 * n + n * logn);
 #if _LIBCPP_HARDENING_MODE != _LIBCPP_HARDENING_MODE_DEBUG
     LIBCPP_ASSERT(stats.compared <= n * logn);
-    (void)debug_comparisons;
 #else
     LIBCPP_ASSERT(stats.compared <= 2 * n * logn + debug_comparisons);
 #endif
diff --git a/libcxx/test/std/containers/sequences/deque/deque.cons/from_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.cons/from_range.pass.cpp
index 0ea7c17d7659c42..cfc07ab7bc797d4 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.cons/from_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.cons/from_range.pass.cpp
@@ -19,7 +19,7 @@
 
 int main(int, char**) {
   for_all_iterators_and_allocators<int>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_container<std::deque, int, Iter, Sent, Alloc>([](const auto& c) {
+    test_sequence_container<std::deque, int, Iter, Sent, Alloc>([]([[maybe_unused]] const auto& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
index e6fe8c8b9e2326a..56a1d226db46f31 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/append_range.pass.cpp
@@ -26,7 +26,7 @@ int main(int, char**) {
   static_assert(test_constraints_append_range<std::deque, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_append_range<std::deque<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_append_range<std::deque<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/assign_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/assign_range.pass.cpp
index b830000518d5454..744e03a7b983e9e 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/assign_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/assign_range.pass.cpp
@@ -25,7 +25,7 @@ int main(int, char**) {
   static_assert(test_constraints_assign_range<std::deque, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_assign_range<std::deque<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_assign_range<std::deque<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/insert_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/insert_range.pass.cpp
index 7d0b3b9db28f80e..a5f5455297ad44e 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/insert_range.pass.cpp
@@ -26,7 +26,7 @@ int main(int, char**) {
   static_assert(test_constraints_insert_range<std::deque, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_insert_range<std::deque<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_insert_range<std::deque<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
index 08c9c02a8b69939..3154cd389d2f0f4 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
@@ -26,7 +26,7 @@ int main(int, char**) {
   static_assert(test_constraints_prepend_range<std::deque, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_prepend_range<std::deque<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_prepend_range<std::deque<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp
index e76f9bcd3110acf..aafeec766944905 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/append_range.pass.cpp
@@ -25,7 +25,7 @@ constexpr bool test() {
   static_assert(test_constraints_append_range<std::vector, bool, char>());
 
   for_all_iterators_and_allocators<bool, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_append_range<std::vector<bool, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_append_range<std::vector<bool, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       // `is_contiguous_container_asan_correct` doesn't work on `vector<bool>`.
     });
diff --git a/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
index 6ae5aa46d6cc142..e5d0454a844d5a8 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/assign_range.pass.cpp
@@ -25,7 +25,7 @@ constexpr bool test() {
   static_assert(test_constraints_assign_range<std::vector, bool, char>());
 
   for_all_iterators_and_allocators<bool, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_assign_range<std::vector<bool, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_assign_range<std::vector<bool, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       // `is_contiguous_container_asan_correct` doesn't work on `vector<bool>`.
     });
diff --git a/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
index d1f0bf06ed57550..03f3100b928833a 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/construct_from_range.pass.cpp
@@ -18,7 +18,7 @@
 
 constexpr bool test() {
   for_all_iterators_and_allocators<bool>([]<class Iter, class Sent, class Alloc>() {
-    test_vector_bool<Iter, Sent, Alloc>([](const auto& c) {
+    test_vector_bool<Iter, Sent, Alloc>([]([[maybe_unused]] const auto& c) {
       LIBCPP_ASSERT(c.__invariants());
       // `is_contiguous_container_asan_correct` doesn't work on `vector<bool>`.
     });
diff --git a/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
index 260a1037173e14b..65d085fa1f08325 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/insert_range.pass.cpp
@@ -25,7 +25,7 @@ constexpr bool test() {
   static_assert(test_constraints_insert_range<std::vector, bool, char>());
 
   for_all_iterators_and_allocators<bool, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_insert_range<std::vector<bool, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_insert_range<std::vector<bool, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       // `is_contiguous_container_asan_correct` doesn't work on `vector<bool>`.
     });
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
index 2acdcc35da6f4f6..5fb2b46f7e94208 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/construct_from_range.pass.cpp
@@ -19,7 +19,7 @@
 
 constexpr bool test() {
   for_all_iterators_and_allocators<int>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_container<std::vector, int, Iter, Sent, Alloc>([](const auto& c) {
+    test_sequence_container<std::vector, int, Iter, Sent, Alloc>([]([[maybe_unused]] const auto& c) {
       LIBCPP_ASSERT(c.__invariants());
       LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
     });
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
index 0a9453428753444..69e6df6fcffa851 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/append_range.pass.cpp
@@ -27,7 +27,7 @@ constexpr bool test() {
   static_assert(test_constraints_append_range<std::vector, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_append_range<std::vector<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_append_range<std::vector<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
     });
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
index 891c6df4474dc02..8ab3dc10aed9902 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/assign_range.pass.cpp
@@ -27,7 +27,7 @@ constexpr bool test() {
   static_assert(test_constraints_assign_range<std::vector, int, double>());
 
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_assign_range<std::vector<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_assign_range<std::vector<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
     });
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
index 3b900ce73e98f56..0e26cb1546277bd 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.modifiers/insert_range.pass.cpp
@@ -26,7 +26,7 @@
 
 constexpr bool test() {
   for_all_iterators_and_allocators<int, const int*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_insert_range<std::vector<int, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_insert_range<std::vector<int, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
       LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
     });
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/append_range.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/append_range.pass.cpp
index 68327f65bdb381a..701282bcefaf711 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_append/append_range.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_append/append_range.pass.cpp
@@ -23,7 +23,7 @@
 
 constexpr bool test_constexpr() {
   for_all_iterators_and_allocators_constexpr<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_append_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_append_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
@@ -35,7 +35,7 @@ int main(int, char**) {
   static_assert(test_constraints_append_range<std::basic_string, char, int>());
 
   for_all_iterators_and_allocators<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_append_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_append_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/assign_range.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/assign_range.pass.cpp
index 5c43eeaf39dfff4..c11a97f3db23579 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/assign_range.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_assign/assign_range.pass.cpp
@@ -23,7 +23,7 @@
 
 constexpr bool test_constexpr() {
   for_all_iterators_and_allocators_constexpr<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_assign_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_assign_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
@@ -35,7 +35,7 @@ int main(int, char**) {
   static_assert(test_constraints_assign_range<std::basic_string, char, int>());
 
   for_all_iterators_and_allocators<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_assign_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_assign_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp b/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
index aefe73a9a507f22..e66a870d0b5e9bc 100644
--- a/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.modifiers/string_insert/insert_range.pass.cpp
@@ -23,7 +23,7 @@
 
 constexpr bool test_constexpr() {
   for_all_iterators_and_allocators_constexpr<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_insert_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_insert_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
@@ -35,7 +35,7 @@ int main(int, char**) {
   static_assert(test_constraints_insert_range<std::basic_string, char, int>());
 
   for_all_iterators_and_allocators<char, const char*>([]<class Iter, class Sent, class Alloc>() {
-    test_sequence_insert_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([](auto&& c) {
+    test_sequence_insert_range<std::basic_string<char, std::char_traits<char>, Alloc>, Iter, Sent>([]([[maybe_unused]] auto&& c) {
       LIBCPP_ASSERT(c.__invariants());
     });
   });
diff --git a/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp b/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
index fba2bd80d7f89a0..2e971a940c509ce 100644
--- a/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
+++ b/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
@@ -38,24 +38,24 @@ int main(int, char**) {
 #if TEST_STD_VER >= 17
   {
     std::future_error const f(std::future_errc::broken_promise);
-    char const* what = f.what();
+    [[maybe_unused]] char const* what = f.what();
     LIBCPP_ASSERT(what == std::string_view{"The associated promise has been destructed prior "
                                            "to the associated state becoming ready."});
   }
   {
     std::future_error f(std::future_errc::future_already_retrieved);
-    char const* what = f.what();
+    [[maybe_unused]] char const* what = f.what();
     LIBCPP_ASSERT(what == std::string_view{"The future has already been retrieved from "
                                            "the promise or packaged_task."});
   }
   {
     std::future_error f(std::future_errc::promise_already_satisfied);
-    char const* what = f.what();
+    [[maybe_unused]] char const* what = f.what();
     LIBCPP_ASSERT(what == std::string_view{"The state of the promise has already been set."});
   }
   {
     std::future_error f(std::future_errc::no_state);
-    char const* what = f.what();
+    [[maybe_unused]] char const* what = f.what();
     LIBCPP_ASSERT(what == std::string_view{"Operation not permitted on an object without "
                                            "an associated state."});
   }
diff --git a/libcxx/test/std/utilities/function.objects/func.invoke/invoke_r.pass.cpp b/libcxx/test/std/utilities/function.objects/func.invoke/invoke_r.pass.cpp
index af4c0baf8c60c59..8a08d3636d1e277 100644
--- a/libcxx/test/std/utilities/function.objects/func.invoke/invoke_r.pass.cpp
+++ b/libcxx/test/std/utilities/function.objects/func.invoke/invoke_r.pass.cpp
@@ -91,7 +91,7 @@ constexpr bool test() {
         {
             bool was_called = false;
             auto f = [&](NonCopyable) -> int { was_called = true; return 0; };
-            std::invoke_r<int>(f, NonCopyable());
+            (void)std::invoke_r<int>(f, NonCopyable());
             assert(was_called);
         }
         // Forward function object, with void return
@@ -111,7 +111,7 @@ constexpr bool test() {
                 constexpr int operator()() && { was_called = true; return 0; }
             };
             bool was_called = false;
-            std::invoke_r<int>(MoveOnlyIntFunction{was_called});
+            (void)std::invoke_r<int>(MoveOnlyIntFunction{was_called});
             assert(was_called);
         }
     }
diff --git a/libcxx/test/support/deduction_guides_sfinae_checks.h b/libcxx/test/support/deduction_guides_sfinae_checks.h
index 8d1365848eafc48..8b715da5a34e25f 100644
--- a/libcxx/test/support/deduction_guides_sfinae_checks.h
+++ b/libcxx/test/support/deduction_guides_sfinae_checks.h
@@ -116,10 +116,10 @@ constexpr void SequenceContainerDeductionGuidesSfinaeAway() {
 template<template<typename ...> class Container, typename InstantiatedContainer>
 constexpr void ContainerAdaptorDeductionGuidesSfinaeAway() {
   using T = typename InstantiatedContainer::value_type;
-  using Alloc = std::allocator<T>;
+  using Alloc [[maybe_unused]] = std::allocator<T>;
   using Iter = T*;
 
-  using BadIter = int;
+  using BadIter [[maybe_unused]] = int;
   using BadAlloc = Empty;
 
   // (container) -- no constraints.

Copy link

github-actions bot commented Nov 26, 2023

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

@philnik777
Copy link
Contributor

Since you mention a few warning flags in the description, I think it would be good to add them to libcxx/utils/libcxx/test/params.py to avoid backsliding.

@StephanTLavavej
Copy link
Member Author

params.py has already opted into -Wunused-variable and -Wunused-parameter, but explicitly opted out with -Wno-unused-local-typedef:

"-Wunused-variable",
"-Wunused-parameter",
"-Wunreachable-code",
"-Wno-unused-local-typedef",

I'm seeing these warnings because of our exotic configuration of using MSVC's STL, so LIBCPP_ASSERT etc. expand to nothing. (And we don't go through libcxx/utils/libcxx/test/params.py, so we get unused-local-typedef warnings despite the opt-out mentioned there.)

To prevent backsliding between microsoft/STL's infrequent updates of its llvm-project submodule, I suppose you could add a test configuration where LIBCPP_ASSERT etc. are forced to expand to nothing despite being used with libc++ product code - but that seems like it would be a lot of effort to find relatively low value issues that are easily fixed.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that makes sense. LGTM.

@philnik777 philnik777 merged commit 346a299 into llvm:main Nov 26, 2023
@StephanTLavavej StephanTLavavej deleted the stl-3-unused-nodiscard-warnings branch November 26, 2023 22:00
StephanTLavavej added a commit that referenced this pull request Dec 10, 2023
Found while running libc++'s tests with MSVC's STL.

*
`libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp`
+ Fix Clang `-Wunused-variable`, because `LIBCPP_ASSERT` expands to
nothing for MSVC's STL.
+ This is the same "always void-cast" change that #73437 applied to the
neighboring `complexity.pass.cpp`. I missed that
`ranges_sort_heap.pass.cpp` was also affected because we had disabled
this test.
*
`libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp`
*
`libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp`
+ Fix MSVC "warning C4244: '`=`': conversion from '`__int64`' to
'`_Ty`', possible loss of data".
+ This is a valid warning, possibly the best one that MSVC found in this
entire saga. We're accumulating a `std::vector<std::streamsize>` and
storing the result in `std::streamsize total_size` but we actually have
to start with `std::streamsize{0}` or we'll truncate.
*
`libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp`
+ Fix Clang `-Wunused-local-typedef` because the following usage is
libc++-only.
+ I'm just expanding it at the point of use, and using the dedicated
`LIBCPP_STATIC_ASSERT` to keep the line length down.
*
`libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp`
+ Fix MSVC "warning C4242: 'argument': conversion from '`int`' to
'`const _Elem`', possible loss of data".
+ This is a valid warning (possibly the second-best) as `sputc()`
returns `int_type`. If `sputc()` returns something unexpected, we want
to know, so we should separately say `expected.push_back(CharT('B'))`.
*
`libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp`
*
`libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp`
  + Fix MSVC "warning C6001: Using uninitialized memory '`x`'."
  + [N4964](https://wg21.link/N4964) \[new.delete.single\]/12:
> *Effects:* The deallocation functions
(\[basic.stc.dynamic.deallocation\]) called by a *delete-expression*
(\[expr.delete\]) to render the value of `ptr` invalid.
  + \[basic.stc.general\]/4:
> When the end of the duration of a region of storage is reached, the
values of all pointers representing the address of any part of that
region of storage become invalid pointer values (\[basic.compound\]).
Indirection through an invalid pointer value and passing an invalid
pointer value to a deallocation function have undefined behavior. Any
other use of an invalid pointer value has implementation-defined
behavior.
+ In certain configurations, after `delete x;` MSVC will consider `x` to
be radioactive (and in other configurations, it'll physically null out
`x` as a safety measure). We can copy it into `old_x` before deletion,
which the implementation finds acceptable.
*
`libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp`
*
`libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp`
+ Fix MSVC "warning C4242: 'initializing': conversion from '`_Ty`' to
'`_Ty`', possible loss of data".
+ This was being emitted in `pair` and `tuple`'s perfect forwarding
constructors. Passing `short{1}` allows MSVC to see that no truncation
is happening.
*
`libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp`
+ Fix MSVC "warning C4242: 'initializing': conversion from '`_Ty`' to
'`_Ty2`', possible loss of data".
+ Similarly, this was being emitted in `pair`'s perfect forwarding
constructor. After passing `short{1}`, I reduced repetition by relying
on CTAD. (I can undo that cleanup if it's stylistically undesirable.)
*
`libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp`
+ Fix MSVC "warning C4930: '`std::reference_wrapper<int> purr(void)`':
prototyped function not called (was a variable definition intended?)".
+ There's no reason for `purr()` to be locally declared (aside from
isolating it to a narrow scope, which has minimal benefits); it can be
declared like `meow()` above. 😸
*
`libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp`
*
`libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp`
  + Fix MSVC static analysis warnings when replacing `operator new`:
    ```
warning C28196: The requirement that '(_Param_(1)>0)?(return!=0):(1)' is
not satisfied. (The expression does not evaluate to true.)
warning C6387: 'return' could be '0': this does not adhere to the
specification for the function 'new'.
warning C6011: Dereferencing NULL pointer 'reinterpret_cast<char
*>ptr+i'.
    ```
+ All we need is a null check, which appears in other `operator new`
replacements:
https://github.com/llvm/llvm-project/blob/b85f1f9b182234ba366d78ae2174a149e44d08c1/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp#L27-L28
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