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

[cmake] Add detection of C++1x features, and store results in SiconosConfig.h #10

Merged
merged 2 commits into from
Feb 2, 2016

Conversation

radarsat1
Copy link
Contributor

This PR includes the changes from PR #9, but proposes a different approach for handling C++1x features. To summarize:

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 caused linker errors.

This patch:

  • Adds detection of individual C++11 features, such as std::shared_ptr, std::array, std::unordered_map, std::to_string, std::bind.
  • Detection variables are stored in SiconosConfig.h
    • e.g., SICONOS_STD_SHARED_PTR, etc..
  • User flags to turn off use of these features are also stored in SiconosConfig.h:
    • SICONOS_USE_BOOST_FOR_CXX11
    • SICONOS_USE_MAP_FOR_HASH
  • 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 specific feature variables.

Note that CMake's own mechanism for setting the C++ standard must be used to select C++11 or C++14:

cmake .. -DCMAKE_CXX_STANDARD=11

This option is explicitly passed on to the detection try_run() calls.

@@ -28,7 +28,7 @@
#include <boost/serialization/serialization.hpp>
#include <boost/serialization/nvp.hpp>

#if __cplusplus >= 201103L
#if SICONOS_CXXVERSION >= 201103L
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This was the only one I wasn't sure to do with in terms of feature detection, so I at least changed the comparison with __cplusplus for SICONOS_CXXVERSION.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this 3.5 years ago (thanks git blame for reminding me this), so I had to take a stab at it and try to remember ... So looking at the code, ser_shared_ptr.hpp does the serialization of the std shared_ptr. Hence, I would add
#if defined(SICONOS_STD_SHARED_PTR) && !defined(SICONOS_USE_BOOST_FOR_CXX11)
here.

@@ -7,7 +7,7 @@

%import SiconosConfig.h

#if (SICONOS_CXXVERSION >= 201103L) && !defined(USE_BOOST_FOR_CXX11)
#if defined(SICONOS_STD_TO_STRING) && !defined(SICONOS_USE_BOOST_FOR_CXX11)
#define STD11 std
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant SICONOS_STD_SHARED_PTR: the following bits of code are from g++-v4/bits/shared_ptr.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, good catch! Must have done that one a little too quickly.

@xhub
Copy link
Member

xhub commented Feb 1, 2016

I like this new version. I just had 2 minors comments. If you think the proposed changes make sense, just commit an updated version to the main repo.

@vacary vacary assigned xhub and bremond and unassigned xhub and bremond Feb 2, 2016
…Config.h.

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 caused linker errors.

This patch:

  * Adds detection of individual C++11 features, such as
    std::shared_ptr, std::array, std::unordered_map, std::to_string,
    std::bind.

  * Detection variables are stored in SiconosConfig.h
    e.g., SICONOS_STD_SHARED_PTR, etc..

  * Detection of individual features is only performed if CXXVERSION
    or CMAKE_CXX_STANDARD is changed.

  * User flags to turn off use of these features are also stored in
    SiconosConfig.h.  They default to ON to keep compatibility with
    previous behaviour.

    - SICONOS_USE_BOOST_FOR_CXX11
    - SICONOS_USE_MAP_FOR_HASH

  * 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 specific feature variables.

Note that CMake's own mechanism for setting the C++ standard must be
used to select C++11 or C++14:

    cmake .. -DCMAKE_CXX_STANDARD=11

This option is explicitly passed on to the try_run() calls used for
feature detection.
…h C++11.

Full warning is,

    warning: invalid suffix on literal; C++11 requires a space between
             literal and string macro [-Wliteral-suffix]

and in clang++, it is an error:

    error: invalid suffix on literal; C++11 requires a space between
           literal and identifier [-Wreserved-user-defined-literal]
@radarsat1
Copy link
Contributor Author

Ok I updated the branch but apparently I don't have push access for the siconos repository.

@xhub
Copy link
Member

xhub commented Feb 2, 2016

I added you to the siconos group. I think that's the way to go.

@radarsat1 radarsat1 merged commit 19b3d91 into siconos:master Feb 2, 2016
@radarsat1
Copy link
Contributor Author

Seems to have worked, thanks!

vacary pushed a commit to vacary/siconos that referenced this pull request Dec 22, 2023
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

3 participants