Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix C++11 support in CMake and headers. #9

Closed
wants to merge 3 commits into from

Conversation

radarsat1
Copy link
Contributor

It seems that compiling software that uses C++11 against Siconos would successfully compile, but not link. It turned out be because, by default, Siconos compiles without -std=c++11, and uses boost::shared_ptr, etc. However, the use of boost::shared_ptr or std::shared_ptr is detected by the preprocessor by checking against __cplusplus in some headers. So software including those headers when -std=c++11 was enabled would incorrectly choose the std:: namespace, when Siconos was compiled with the boost:: namespace.

This patch series:

  • Adds USE_CXX11 to CMake, which requests -std=c++11 enabled, and checks that it works during configuration. An error is emitted if it is requested but not supported. It is disabled by default.
  • Fixes some problematic areas in mechanics where boost::array or similar was used even when c++11 was enabled.
  • Replaces checks against __cplusplus in the headers with checks against SICONOS_CXXVERSION, which is hardcoded at configure time, guaranteeing that user software will select the shared_ptr type that Siconos was compiled with.

Without this, some implementation files do not compile when USE_CXX1
is enabled.
…headers.

The C++ version is used to select between C++11 and Boost shared_ptr
definitions, which changes the ABI.  Therefore it is necessary for
external codebases using the Siconos headers to define the same
interface.  SICONOS_CXXVERSION is a copy of __cplusplus defined at
configure time, and therefore is the one used to compile Siconos.

Without this patch, C++11 code using Siconos compiled with C++98 will
result in a successful compilation, but mysteriously fail to link.
@xhub
Copy link
Member

xhub commented Jan 28, 2016

Good idea!

@radarsat1
Copy link
Contributor Author

Okay I have been looking a little deeper into CMake support for C++11, and here's a slightly different proposal:

  • I tried to "do the right thing" and use CMAKE_CXX_STANDARD and module WriteCompilerDetectionHeader along with target_compile_features() for detecting and specifying specific C++ features. CMake is then responsible for determining the C++ standard to use and how to use it.
  • However, I discovered that mysteriously CMake doesn't provide any feature named "cxx_memory_shared_ptr" or similar -- so it's a bit of a dead-end to let CMake take care of this.

However, the idea of detecting specific features is good. So, I propose:

  • User can set CMAKE_CXX_STANDARD and USE_BOOST_FOR_CXX11 to whatever he wants.
    • Therefore we presume he knows that enabling CMAKE_CXX_STANDARD=11 will enable namespace SP=std, unless USE_BOOST_FOR_CXX11 is enabled.
  • If USE_BOOST_FOR_CXX11 not set, check if std::shared_ptr is supported.
    • If yes, use namespace SP=std and set a proprocessor variable SICONOS_STD_SHARED_PTR
    • Otherwise, do not use it.
  • We mostly forget about SICONOS_CXXVERSION, and use feature variables in all cases.
  • Variables SICONOS_STD_SHARED_PTR and SICONOS_USE_BOOST_FOR_CXX11 are recorded to SiconosConfig.h and used in headers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants