Skip to content

[libc++][test] Make macro detection more friendly to MSVC#113633

Merged
frederick-vs-ja merged 1 commit intollvm:mainfrom
frederick-vs-ja:msvc-long-double
Oct 26, 2024
Merged

[libc++][test] Make macro detection more friendly to MSVC#113633
frederick-vs-ja merged 1 commit intollvm:mainfrom
frederick-vs-ja:msvc-long-double

Conversation

@frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Oct 25, 2024 •
edited
Loading

MSVC STL's test suite is a bit nervous about replacing non-macro-defined identifiers with 0 (see also https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4668?view=msvc-170).

On MSVC (and MS-compatible mode of other compilers), long double has the same format (IEEE-754 binary64) as double, so it should be OK to define TEST_LONG_DOUBLE_IS_DOUBLE when _MSC_VER is defined. Such detection should be performed first.

@frederick-vs-ja
[libc++][test] Make macro detection more friendly to MSVC
c04730f 
MSVC's test suite is a bit nervous about replacing non-macro-defined identifiers with `0` (see also https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4668?view=msvc-170).

On MSVC (and MS-compatible mode of other compilers), `long double` has the same format (IEEE-754 binary64) as  `double`, so it should be OK to define `TEST_LONG_DOUBLE_IS_DOUBLE` when `_MSC_VER` is defined. Such detection should be performed first.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner October 25, 2024 02:19
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 25, 2024

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

MSVC STL's test suite is a bit nervous about replacing non-macro-defined identifiers with 0 (see also https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4668?view=msvc-170).

On MSVC (and MS-compatible mode of other compilers), long double has the same format (IEEE-754 binary64) as double, so it should be OK to define TEST_LONG_DOUBLE_IS_DOUBLE when _MSC_VER is defined. Such detection should be performed first.

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

1 Files Affected:

  • (modified) libcxx/test/support/test_macros.h (+1-1)
diff --git a/libcxx/test/support/test_macros.h b/libcxx/test/support/test_macros.h
index 5ef14e54dae237..1b6473b623c53b 100644
--- a/libcxx/test/support/test_macros.h
+++ b/libcxx/test/support/test_macros.h
@@ -511,7 +511,7 @@ inline Tp const& DoNotOptimize(Tp const& value) {
 #  define TEST_CONSTEXPR_OPERATOR_NEW
 #endif
 
-#if __SIZEOF_LONG_DOUBLE__ == __SIZEOF_DOUBLE__
+#if defined(_MSC_VER) || __SIZEOF_LONG_DOUBLE__ == __SIZEOF_DOUBLE__
 #  define TEST_LONG_DOUBLE_IS_DOUBLE
 #endif

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 25, 2024 •
edited
Loading

This looks good to me.

Edit: EDG on Windows doesn't understand these macros, so _MSC_VER is proper to use here IMO; TEST_COMPILER_MSVC would miss EDG.

C:\Temp>type meow.cpp

#include <print>
using namespace std;

int main() {
    println("__SIZEOF_LONG_DOUBLE__: {}", __SIZEOF_LONG_DOUBLE__);
    println("__SIZEOF_DOUBLE__: {}", __SIZEOF_DOUBLE__);
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow
meow.cpp
meow.cpp(5): error C2065: '__SIZEOF_LONG_DOUBLE__': undeclared identifier
meow.cpp(6): error C2065: '__SIZEOF_DOUBLE__': undeclared identifier

C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest meow.cpp && meow
__SIZEOF_LONG_DOUBLE__: 8
__SIZEOF_DOUBLE__: 8

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c /BE meow.cpp
meow.cpp
"meow.cpp", line 5: error: identifier "__SIZEOF_LONG_DOUBLE__" is undefined
      println("__SIZEOF_LONG_DOUBLE__: {}", __SIZEOF_LONG_DOUBLE__);
                                            ^

"meow.cpp", line 5: error: no instance of overloaded function "println" matches
          the argument list
            argument types are: (const char [27], <error type>)
      println("__SIZEOF_LONG_DOUBLE__: {}", __SIZEOF_LONG_DOUBLE__);
      ^

"meow.cpp", line 6: error: identifier "__SIZEOF_DOUBLE__" is undefined
      println("__SIZEOF_DOUBLE__: {}", __SIZEOF_DOUBLE__);
                                       ^

"meow.cpp", line 6: error: no instance of overloaded function "println" matches
          the argument list
            argument types are: (const char [22], <error type>)
      println("__SIZEOF_DOUBLE__: {}", __SIZEOF_DOUBLE__);
      ^

@StephanTLavavej StephanTLavavej mentioned this pull request Oct 25, 2024
Merged
@frederick-vs-ja frederick-vs-ja merged commit 3fc0d94 into llvm:main Oct 26, 2024
@frederick-vs-ja frederick-vs-ja deleted the msvc-long-double branch October 26, 2024 05:47
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
@frederick-vs-ja @winner245
[libc++][test] Make macro detection more friendly to MSVC (llvm#113633)
3a0d0af 
MSVC STL's test suite is a bit nervous about replacing non-macro-defined
identifiers with `0` (see also
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4668?view=msvc-170).

On MSVC (and MS-compatible mode of other compilers), `long double` has
the same format (IEEE-754 binary64) as `double`, so it should be OK to
define `TEST_LONG_DOUBLE_IS_DOUBLE` when `_MSC_VER` is defined. Such
detection should be performed first.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
@frederick-vs-ja @NoumanAmir657
[libc++][test] Make macro detection more friendly to MSVC (llvm#113633)
954d3d5 
MSVC STL's test suite is a bit nervous about replacing non-macro-defined
identifiers with `0` (see also
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4668?view=msvc-170).

On MSVC (and MS-compatible mode of other compilers), `long double` has
the same format (IEEE-754 binary64) as `double`, so it should be OK to
define `TEST_LONG_DOUBLE_IS_DOUBLE` when `_MSC_VER` is defined. Such
detection should be performed first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

@cjdb cjdb cjdb approved these changes

Assignees

No one assigned

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@frederick-vs-ja @llvmbot @StephanTLavavej @cjdb