kou commented on code in PR #43706:
URL: https://github.com/apache/arrow/pull/43706#discussion_r1718264097
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1355,12 +1355,22 @@ macro(build_snappy)
"-DCMAKE_INSTALL_PREFIX=${SNAPPY_PREFIX}")
# Snappy unconditionally enables -Werror when building with clang this can
lead
# to build failures by way of new compiler warnings. This adds a flag to
disable
- # Werror to the very end of the invocation to override the snappy internal
setting.
+ # -Werror to the very end of the invocation to override the snappy internal
setting.
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "")
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS}
-Wno-error")
+ endif()
+ # Snappy unconditionally disables RTTI, which is incompatible with some other
+ # build settings (https://github.com/apache/arrow/issues/43688).
+ if(NOT MSVC)
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -frtti")
+ endif()
+
+ if(NOT MSVC)
Review Comment:
Can we remove this `if()`?
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1355,12 +1355,22 @@ macro(build_snappy)
"-DCMAKE_INSTALL_PREFIX=${SNAPPY_PREFIX}")
# Snappy unconditionally enables -Werror when building with clang this can
lead
# to build failures by way of new compiler warnings. This adds a flag to
disable
- # Werror to the very end of the invocation to override the snappy internal
setting.
+ # -Werror to the very end of the invocation to override the snappy internal
setting.
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "")
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS}
-Wno-error")
Review Comment:
```suggestion
string(APPEND SNAPPY_ADDITIONAL_CXX_FLAGS " -Wno-error")
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1355,12 +1355,22 @@ macro(build_snappy)
"-DCMAKE_INSTALL_PREFIX=${SNAPPY_PREFIX}")
# Snappy unconditionally enables -Werror when building with clang this can
lead
# to build failures by way of new compiler warnings. This adds a flag to
disable
- # Werror to the very end of the invocation to override the snappy internal
setting.
+ # -Werror to the very end of the invocation to override the snappy internal
setting.
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "")
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS}
-Wno-error")
+ endif()
+ # Snappy unconditionally disables RTTI, which is incompatible with some other
+ # build settings (https://github.com/apache/arrow/issues/43688).
+ if(NOT MSVC)
+ set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -frtti")
Review Comment:
```suggestion
string(APPEND SNAPPY_ADDITIONAL_CXX_FLAGS " -frtti")
```
--
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]
