[libcxx-commits] [libcxx] [libc++][test] Fix more MSVC and Clang warnings (PR #74965)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

<details>
<summary>Changes</summary>

Found while running libc++'s tests with MSVC's STL.

As usual, I'm trying to strike a balance between avoiding "grab bag" PRs and creating a trillion PRs. After gathering the `static_cast` warning fixes in #<!-- -->74962, I believe that these changes are small enough to group together. Please let me know if you'd like me to decompose this further.

* `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. :smile_cat:
* `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


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


13 Files Affected:

- (modified) libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp (+1-1) 
- (modified) libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp (+2-2) 
- (modified) libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp (+2-2) 
- (modified) libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp (+1-3) 
- (modified) libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp (+2-1) 
- (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp (+2-1) 
- (modified) libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp (+2-1) 
- (modified) libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp (+1-1) 
- (modified) libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp (+2-2) 
- (modified) libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp (+1-1) 
- (modified) libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp (+2-1) 
- (modified) libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp (+3) 
- (modified) libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp (+3) 


``````````diff
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp
index 1153ed573d635f..1e636ea9afac47 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.heap.operations/sort.heap/ranges_sort_heap.pass.cpp
@@ -238,6 +238,7 @@ void test_complexity() {
     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, &MyInt::Comp);
     // The exact stats of our current implementation are recorded here.
@@ -247,7 +248,6 @@ void test_complexity() {
     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/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp
index d57b7c20a2da27..ecc11f4999ffa1 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/buffered_reads.pass.cpp
@@ -44,7 +44,7 @@
 
 template <class BufferPolicy>
 void test_read(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
-  std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
+  std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), std::streamsize{0});
   std::vector<char> data(total_size);
   for (std::size_t i = 0; i < data.size(); ++i) {
     data[i] = static_cast<char>(i % (1 << 8 * sizeof(char)));
@@ -99,7 +99,7 @@ void test_read(BufferPolicy policy, const std::vector<std::streamsize>& payload_
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
 template <class BufferPolicy>
 void test_read_codecvt(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
-  std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
+  std::streamsize total_size = std::accumulate(payload_sizes.begin(), payload_sizes.end(), std::streamsize{0});
   std::vector<wchar_t> data(total_size);
   for (std::size_t i = 0; i < data.size(); ++i) {
     data[i] = static_cast<wchar_t>(i);
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp
index e7820739505106..b5bbb0ca2ee4e7 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/ofstream.members/buffered_writes.pass.cpp
@@ -45,7 +45,7 @@
 template <class BufferPolicy>
 void test_write(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
   std::size_t previously_written = 0;
-  std::streamsize total_size     = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
+  std::streamsize total_size     = std::accumulate(payload_sizes.begin(), payload_sizes.end(), std::streamsize{0});
   std::vector<char> data(total_size);
   for (std::size_t i = 0; i < data.size(); ++i) {
     data[i] = static_cast<char>(i % (1 << 8 * sizeof(char)));
@@ -97,7 +97,7 @@ void test_write(BufferPolicy policy, const std::vector<std::streamsize>& payload
 template <class BufferPolicy>
 void test_write_codecvt(BufferPolicy policy, const std::vector<std::streamsize>& payload_sizes) {
   std::size_t previously_written = 0;
-  std::streamsize total_size     = std::accumulate(payload_sizes.begin(), payload_sizes.end(), 0);
+  std::streamsize total_size     = std::accumulate(payload_sizes.begin(), payload_sizes.end(), std::streamsize{0});
   std::vector<wchar_t> data(total_size);
   for (std::size_t i = 0; i < data.size(); ++i) {
     data[i] = static_cast<wchar_t>(i);
diff --git a/libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp
index 7f3022a3ce9bdd..ad0cdb092def8f 100644
--- a/libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp
@@ -25,9 +25,7 @@ int main(int, char**) {
   typedef fs::path::format E;
   static_assert(std::is_enum<E>::value, "");
 
-  typedef std::underlying_type<E>::type UT;
-
-  LIBCPP_ONLY(static_assert(std::is_same<UT, unsigned char>::value, "")); // Implementation detail
+  LIBCPP_STATIC_ASSERT(std::is_same<std::underlying_type<E>::type, unsigned char>::value, ""); // Implementation detail
 
   static_assert(
           E::auto_format   != E::native_format &&
diff --git a/libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp b/libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp
index ba007da5a054a7..a236bf4752a076 100644
--- a/libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp
+++ b/libcxx/test/std/input.output/syncstream/syncbuf/syncstream.syncbuf.assign/swap.pass.cpp
@@ -75,7 +75,8 @@ static void test_short_write_after_swap() {
     sync_buf2.sputn(expected.data(), expected.size());
 
     sync_buf1.swap(sync_buf2);
-    expected.push_back(sync_buf1.sputc(CharT('B')));
+    sync_buf1.sputc(CharT('B'));
+    expected.push_back(CharT('B'));
     sync_buf2.sputc(CharT('Z'));
 
     assert(sstr1.str().empty());
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
index 4e5d36cd7c6dfc..1c575729678d5c 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.pass.cpp
@@ -60,8 +60,9 @@ int main(int, char**) {
         assert(reinterpret_cast<std::uintptr_t>(x) % alignof(TrackLifetimeOverAligned) == 0);
         assert(info.address_constructed == x);
 
+        const auto old_x = x;
         delete x;
-        assert(info.address_destroyed == x);
+        assert(info.address_destroyed == old_x);
     }
 
     return 0;
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp
index 398de0068aba1b..56ae8df43f66fb 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.pass.cpp
@@ -50,8 +50,9 @@ int main(int, char**) {
         assert(x != nullptr);
         assert(info.address_constructed == x);
 
+        const auto old_x = x;
         delete x;
-        assert(info.address_destroyed == x);
+        assert(info.address_destroyed == old_x);
     }
 
     return 0;
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp
index 78792ae54bdbfc..d5318ced73dcd9 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/general.pass.cpp
@@ -70,7 +70,7 @@ int main(int, char**) {
 
   // tuple
   {
-    std::tuple<short> tps[] = {{1}, {2}, {3}};
+    std::tuple<short> tps[] = {{short{1}}, {short{2}}, {short{3}}};
     auto ev                 = tps | std::views::elements<0>;
     auto expected           = {1, 2, 3};
     assert(std::ranges::equal(ev, expected));
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp
index d87a3e53392036..f88091f42699e1 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/deref.pass.cpp
@@ -50,7 +50,7 @@ constexpr void testValue(T t) {
 constexpr bool test() {
   // test tuple
   {
-    std::tuple<int, short, long> ts[] = {{1, 2, 3}, {4, 5, 6}};
+    std::tuple<int, short, long> ts[] = {{1, short{2}, 3}, {4, short{5}, 6}};
     testReference<0>(ts);
     testReference<1>(ts);
     testReference<2>(ts);
@@ -61,7 +61,7 @@ constexpr bool test() {
 
   // test pair
   {
-    std::pair<int, short> ps[] = {{1, 2}, {4, 5}};
+    std::pair<int, short> ps[] = {{1, short{2}}, {4, short{5}}};
     testReference<0>(ps);
     testReference<1>(ps);
     testValue<0>(ps[0]);
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp
index 9a76c2fcb70c24..6fec655c67810a 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/member_types.compile.pass.cpp
@@ -65,7 +65,7 @@ static_assert(std::same_as<ElementsIter<Range<std::tuple<int>*>>::iterator_categ
                            std::random_access_iterator_tag>);
 
 using Generator = decltype(std::views::iota(0, 1) | std::views::transform([](int) {
-                             return std::pair<int, short>{1, 1};
+                             return std::pair{1, short{1}};
                            }));
 static_assert(std::ranges::random_access_range<Generator>);
 
diff --git a/libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp b/libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp
index 2759b921fabe0c..3ff8a872095135 100644
--- a/libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp
+++ b/libcxx/test/std/utilities/function.objects/refwrap/refwrap.const/type_conv_ctor.pass.cpp
@@ -37,6 +37,8 @@ struct convertible_from_int {
 void meow(std::reference_wrapper<int>) {}
 void meow(convertible_from_int) {}
 
+std::reference_wrapper<int> purr();
+
 int main(int, char**)
 {
   {
@@ -58,7 +60,6 @@ int main(int, char**)
     meow(0);
   }
   {
-    extern std::reference_wrapper<int> purr();
     ASSERT_SAME_TYPE(decltype(true ? purr() : 0), int);
   }
 #if TEST_STD_VER > 14
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp
index 21e1786f015882..ce8d91d4e26e6b 100644
--- a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared_for_overwrite.pass.cpp
@@ -60,6 +60,9 @@ constexpr char pattern = 0xDE;
 
 void* operator new(std::size_t count) {
   void* ptr = std::malloc(count);
+  if (!ptr) {
+    std::abort(); // placate MSVC's unchecked malloc warning
+  }
   for (std::size_t i = 0; i < count; ++i) {
     *(reinterpret_cast<char*>(ptr) + i) = pattern;
   }
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp
index 8011a37be08ecd..5954703109257d 100644
--- a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.default_init.pass.cpp
@@ -27,6 +27,9 @@ constexpr char pattern = 0xDE;
 
 void* operator new(std::size_t count) {
   void* ptr = std::malloc(count);
+  if (!ptr) {
+    std::abort(); // placate MSVC's unchecked malloc warning
+  }
   for (std::size_t i = 0; i < count; ++i) {
     *(reinterpret_cast<char*>(ptr) + i) = pattern;
   }

``````````

</details>


https://github.com/llvm/llvm-project/pull/74965