RosettaCommons / binder

Binder, tool for automatic generation of Python bindings
MIT License
317 stars 67 forks source link

LLVM >= 16.0.6 no longer builds with --std=c++14 #264

Closed andrewandersonintel closed 1 year ago

andrewandersonintel commented 1 year ago

Hello,

When trying to rebuild a binder-based project that had been working fine for many months, I've discovered that it no longer builds. The trigger for the failure was updating LLVM. Looking at the error messages, it seems that binder can't rely on SET(CMAKE_CXX_STANDARD 14) any more, since LLVM now relies on a large amount of C++17.

I see tens of thousands of lines of error messages, but they're all of this form:

/usr/include/llvm/ADT/STLForwardCompat.h:46:35: error: ‘optional’ in namespace ‘std’ does not name a template type
   46 | auto transformOptional(const std::optional<T> &O, const Function &F)
      |                                   ^~~~~~~~
/usr/include/llvm/ADT/STLForwardCompat.h:46:30: note: ‘std::optional’ is only available from C++17 onwards

I have tried to override the setting for CMAKE_CXX_STANDARD in my ExternalProject_add invocation by using the CMAKE_ARGS parameter, but since it is SET and not OPTION in the binder CMakeLists.txt this has no effect.

ExternalProject_Add(binder
    GIT_REPOSITORY git@github.com:RosettaCommons/binder.git
    SOURCE_DIR ${CMAKE_BINARY_DIR}/binder/src
    BINARY_DIR ${CMAKE_BINARY_DIR}/binder/build
    INSTALL_DIR ${CMAKE_BINARY_DIR}/binder/install
    CMAKE_ARGS
        -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>
        -DCMAKE_CXX_STANDARD=17
        -DBINDER_ENABLE_TEST=OFF)

Does anyone know of a workaround for this / can anyone confirm this issue with LLVM 16+? The only immediate solution I can see is to fork binder and remove the forced C++14 in the CMakeLists.txt but I really don't want to do that!

lyskov commented 1 year ago

Thank you for report @andrewandersonintel ! - have you tried making line SET(CMAKE_CXX_STANDARD 14) conditional depending on LLVM version? Ie, if LLVM version below ~16 then use C++14 otherwise use C++17? (and if that works I will be happy to accept this as PR)

Also tagging @andriish who might have thoughts on this.

andrewandersonintel commented 1 year ago

I'll give it a try and open a PR if it works!

andrewandersonintel commented 1 year ago

Success -- with CMAKE_CXX_STANDARD 17 binder builds successfully against LLVM 16. I opened a PR here https://github.com/RosettaCommons/binder/pull/265

lyskov commented 1 year ago

Thank you @andrewandersonintel ! - PR looks good to me and i just merged it. Closing this issue as resolved.