Skip to content

[libc++][test] Fix more MSVC and Clang warnings #74965

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 13 commits into from
Dec 10, 2023

Conversation

StephanTLavavej
Copy link
Member

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 [libc++][test] Fix unused and nodiscard warnings #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 [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:
      void* ret = std::malloc(s);
      if (!ret) std::abort(); // placate MSVC's unchecked malloc warning

…sible loss of data".

This is a valid warning. 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.
…_Elem', possible loss of data".

This is a valid warning as sputc() returns int_type. If sputc() returns something unexpected, we want to know, so we should separately say expected.push_back(CharT('B')).
…y2', possible loss of data".

This was being emitted in pair's perfect forwarding constructor. It's simple to start with the desired types and rely on CTAD.
…y', possible loss of data".

This was being emitted in pair and tuple's perfect forwarding constructors.
This warning is valid-ish. N4964 [new.delete.single]/12: "Effects: The deallocation functions (6.7.5.5.3) called by a delete-expression (7.6.2.9) 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 (6.8.4). 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`). We can copy it into `old_x` before deletion, which the implementation finds acceptable.
This emitted a bunch of warnings:

make_shared_for_overwrite.pass.cpp(61) : warning C28196: The requirement that '(_Param_(1)>0)?(return!=0):(1)' is not satisfied. (The expression does not evaluate to true.): Lines: 62, 63, 64, 63, 66
make_shared_for_overwrite.pass.cpp(61) : warning C6387: 'return' could be '0':  this does not adhere to the specification for the function 'new'. See line 64 for an earlier location where this can occur: Lines: 62, 63, 64, 63, 66, 61
make_shared_for_overwrite.pass.cpp(64) : warning C6011: Dereferencing NULL pointer 'reinterpret_cast<char *>ptr+i'. : Lines: 62, 63, 64

All we need is a null check, which appears in other `operator new` replacements.
…ototyped 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); it can be declared like meow() above.
I'm just expanding it at the point of use, and using the dedicated LIBCPP_STATIC_ASSERT to keep the line length down.
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner December 10, 2023 04:59
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2023

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes

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&lt;std::streamsize&gt; 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 [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&lt;int&gt; 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)&gt;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&lt;char *&gt;ptr+i'.
      
    • All we need is a null check, which appears in other operator new replacements:
      void* ret = std::malloc(s);
      if (!ret) std::abort(); // placate MSVC's unchecked malloc warning

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 --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;
   }

Copy link

github-actions bot commented Dec 10, 2023

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

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. LGTM modulo some comments.

clang-format the file.

Remove braces around the `meow` and `ASSERT_SAME_TYPE` lines,
as we don't need to isolate anything by introducing scopes.

Add braces to cleanly separate `i` and `ri` from `j` and `rj`,
as there is always a low risk of unintentional mixups with such names.
StephanTLavavej added a commit that referenced this pull request Dec 10, 2023
…W_ASSERT(...)` (#74967)

This is a syntax cleanup, not needed for running libc++'s tests with
MSVC's STL.

While changing
`libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp`
in #74965, I noticed that libc++'s tests almost always use the
special-purpose macros for "this is a libc++-specific `static_assert`
etc." defined by:

https://github.com/llvm/llvm-project/blob/b85f1f9b182234ba366d78ae2174a149e44d08c1/libcxx/test/support/test_macros.h#L240-L253

However, there were a very small number of occurrences that were using
the general-purpose `LIBCPP_ONLY` macro when they could have been using
the special-purpose macros. I believe that they should be cleaned up, to
make it easier to search for usage, and to make it clearer when the full
power of `LIBCPP_ONLY` is necessary.

This is a pure regex replacement from
`LIBCPP_ONLY\((assert|static_assert|ASSERT_NOEXCEPT|ASSERT_NOT_NOEXCEPT)\((.*)\)\);`
to `LIBCPP_\U$1($2);` using the power of [VSCode's case changing in
regex
replace](https://code.visualstudio.com/docs/editor/codebasics#_case-changing-in-regex-replace).

To avoid merge conflicts, this isn't changing the line in
`libcxx/test/std/input.output/filesystems/fs.enum/enum.path.format.pass.cpp`
that #74965 is already changing to use `LIBCPP_STATIC_ASSERT`.
@StephanTLavavej
Copy link
Member Author

Thanks! The checks are green (with only the usual sporadic timeouts/failures and recently fixed failure, plus stage3 generic-msan queued for over 6 hours) after addressing your comments with very simple commits, so I'll go ahead and merge this.

@StephanTLavavej StephanTLavavej merged commit b2cc4b9 into llvm:main Dec 10, 2023
@StephanTLavavej StephanTLavavej deleted the stl-26-even-more-warnings branch December 10, 2023 21:46
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