HuaHuaY commented on code in PR #50979:
URL: https://github.com/apache/arrow/pull/50979#discussion_r3850221808
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -218,22 +218,11 @@ if(WIN32)
set(CXX_COMMON_FLAGS "/W3 /EHsc")
endif()
- # Disable C5105 (macro expansion producing 'defined' has undefined
- # behavior) warning because there are codes that produce this
- # warning in Windows Kits. e.g.:
- #
- # #define _CRT_INTERNAL_NONSTDC_NAMES
\
- # (
\
- # ( defined _CRT_DECLARE_NONSTDC_NAMES &&
_CRT_DECLARE_NONSTDC_NAMES) || \
- # (!defined _CRT_DECLARE_NONSTDC_NAMES && !__STDC__
) \
- # )
+ # Enable the conforming preprocessor to support C++20 features.
#
# See also:
- # * C5105:
https://docs.microsoft.com/en-US/cpp/error-messages/compiler-warnings/c5105
- # * Related reports:
- # *
https://developercommunity.visualstudio.com/content/problem/387684/c5105-with-stdioh-and-experimentalpreprocessor.html
- # *
https://developercommunity.visualstudio.com/content/problem/1249671/stdc17-generates-warning-compiling-windowsh.html
- set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /wd5105")
+ # *
https://devblogs.microsoft.com/cppblog/announcing-full-support-for-a-c-c-conformant-preprocessor-in-msvc/
+ set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /Zc:preprocessor")
Review Comment:
Yes. I also think so. Based on the reference links in the original comments,
I think if we want to remove `/wd5105`, we should upgrade the version of
windows' SDK instead of using new preprocessor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
