kou commented on a change in pull request #7459:
URL: https://github.com/apache/arrow/pull/7459#discussion_r441221973



##########
File path: cpp/cmake_modules/SetupCxxFlags.cmake
##########
@@ -62,12 +62,15 @@ endif()
 # Support C11
 set(CMAKE_C_STANDARD 11)
 
-# This ensures that things like gnu++11 get passed correctly
-set(CMAKE_CXX_STANDARD 11)
+# This ensures that things like c++11 get passed correctly
+set(CMAKE_CXX_STANDARD ${ARROW_CXX_STANDARD})

Review comment:
       How about using `CMAKE_CXX_STANDARD` directly instead of introducing 
`ARROW_CXX_STANDARD`?
   
   ```diff
   diff --git a/cpp/cmake_modules/DefineOptions.cmake 
b/cpp/cmake_modules/DefineOptions.cmake
   index 660caf91c..f916781d7 100644
   --- a/cpp/cmake_modules/DefineOptions.cmake
   +++ b/cpp/cmake_modules/DefineOptions.cmake
   @@ -82,8 +82,6 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL 
"${CMAKE_CURRENT_SOURCE_DIR}")
    
      define_option_string(ARROW_CXXFLAGS "Compiler flags to append when 
compiling Arrow" "")
    
   -  define_option(ARROW_CXX_STANDARD "C++ standard to target (default C++11)" 
11)
   -
      define_option(ARROW_BUILD_STATIC "Build static libraries" ON)
    
      define_option(ARROW_BUILD_SHARED "Build shared libraries" ON)
   diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake 
b/cpp/cmake_modules/SetupCxxFlags.cmake
   index bf7a60138..ac7518eba 100644
   --- a/cpp/cmake_modules/SetupCxxFlags.cmake
   +++ b/cpp/cmake_modules/SetupCxxFlags.cmake
   @@ -63,7 +63,9 @@ endif()
    set(CMAKE_C_STANDARD 11)
    
    # This ensures that things like c++11 get passed correctly
   -set(CMAKE_CXX_STANDARD ${ARROW_CXX_STANDARD})
   +if(NOT DEFINED CMAKE_CXX_STANDARD)
   +  set(CMAKE_CXX_STANDARD 11)
   +endif()
    
    # We require a C++11 compliant compiler
    set(CMAKE_CXX_STANDARD_REQUIRED ON)
   ```
   
   The https://github.com/apache/arrow/pull/7459#issuecomment-645047309 error 
will be fixed.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to