Skip to content

Conversation

@StephanTLavavej
Copy link
Member

This fixes MSVC warning C4127: conditional expression is constant.

Testing TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED by itself doesn't emit this warning, but the condition here is more complicated. I'm expanding the macro and mechanically simplifying the resulting code.

(Yeah, this warning is often annoying, and I introduced TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED to avoid this warning elsewhere, so it's disappointing that it doesn't make the compiler happy here. If this change is undesirable, I can replace it with ADDITIONAL_COMPILE_FLAGS(cl-style-warnings), but ideally I'd like to avoid having to suppress it.)

Testing TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED by itself doesn't
emit this warning, but the condition here is more complicated. I'm
expanding the macro and mechanically simplifying the resulting code.
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner January 29, 2024 07:35
@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

This fixes MSVC warning C4127: conditional expression is constant.

Testing TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED by itself doesn't emit this warning, but the condition here is more complicated. I'm expanding the macro and mechanically simplifying the resulting code.

(Yeah, this warning is often annoying, and I introduced TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED to avoid this warning elsewhere, so it's disappointing that it doesn't make the compiler happy here. If this change is undesirable, I can replace it with ADDITIONAL_COMPILE_FLAGS(cl-style-warnings), but ideally I'd like to avoid having to suppress it.)


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

1 Files Affected:

  • (modified) libcxx/test/std/containers/sequences/array/array.cons/initialization.pass.cpp (+4-1)
diff --git a/libcxx/test/std/containers/sequences/array/array.cons/initialization.pass.cpp b/libcxx/test/std/containers/sequences/array/array.cons/initialization.pass.cpp
index 7991d4738d9699..acd176cce91d9e 100644
--- a/libcxx/test/std/containers/sequences/array/array.cons/initialization.pass.cpp
+++ b/libcxx/test/std/containers/sequences/array/array.cons/initialization.pass.cpp
@@ -28,7 +28,10 @@ struct test_initialization {
             // Before C++20, default initialization doesn't work inside constexpr for
             // trivially default constructible types. This only apply to non-empty arrays,
             // since empty arrays don't hold an element of type T.
-            if (TEST_STD_AT_LEAST_20_OR_RUNTIME_EVALUATED || !std::is_trivially_default_constructible<T>::value) {
+#if TEST_STD_VER < 20
+            if (!TEST_IS_CONSTANT_EVALUATED || !std::is_trivially_default_constructible<T>::value)
+#endif
+            {
                 std::array<T, 1> a1; (void)a1;
                 std::array<T, 2> a2; (void)a2;
                 std::array<T, 3> a3; (void)a3;

@github-actions
Copy link

github-actions bot commented Jan 29, 2024

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

@StephanTLavavej StephanTLavavej merged commit 1c7607e into llvm:main Mar 9, 2024
@StephanTLavavej StephanTLavavej deleted the conditional-expression-is-constant branch March 9, 2024 10:32
@StephanTLavavej
Copy link
Member Author

Merged as all checks passed, except for a surely-unrelated XPASS in one configuration of std/experimental/simd/simd.class/simd_ctor_conversion.pass.cpp. Thanks! 😻

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.

4 participants