C++ visibility support #231
Conversation
| ################################################# | ||
| # Macro to check for visibility capability in compiler | ||
| # Original idea from: https://gitorious.org/ferric-cmake-stuff/ | ||
| macro (check_gcc_visibility) |
There was a problem hiding this comment.
This also works for clang (and maybe other compilers), so maybe rename to check_compiler_visibility?
| */ | ||
|
|
||
| #if defined _WIN32 || defined __CYGWIN__ | ||
| #ifdef BUILDING_DLL |
There was a problem hiding this comment.
In Gazebo, the usage of the same BUILDING_DLL macro for different libraries created several problems (see https://bitbucket.org/osrf/gazebo/issues/2262/symbol-visibility-issues-in-windows). I would recommend to use a fcl-specific macro and set the DEFINE_SYMBOL property of the library appropriately.
There was a problem hiding this comment.
The header in this form is breaking the static builds in MSVC, that is the default behaviour when building the library in MSVC.
There was a problem hiding this comment.
Oh sorry forgot about this issue, thanks for the reminder Silvio. Could please give a look at this patch j-rivero@e509a3d and let me know if it is correct? Unfortunately I don't have a Windows/MSVC15 setup at hand to test.
There was a problem hiding this comment.
Commented in j-rivero@e509a3d .
There was a problem hiding this comment.
Sorry, I actually commented in e509a3d .
| @@ -52,9 +52,9 @@ namespace detail | |||
| //static const size_t EPA_MAX_ITERATIONS = 255; | |||
| // TODO(JS): remove? | |||
|
|
|||
| /// @brief class for EPA algorithm | |||
| /// @brief class FCL_VISIBLE for EPA algorithm | |||
There was a problem hiding this comment.
There are a few spurious insertion of the FCL_VISIBLE macro in the documentation, they can be easily found by searching for FCL_VISIBLE for.
|
Ouk, sorry I've pushed the mess of my working copy to the repo (reverts on reverts). I'll open a new pull request with a clean history and close this one. |
|
Excuse me for the change of PR. Let's follow in #233 |
This PR implements the control of binary symbols visibility in FCL.
It adds a new
FCL_VISIBLEpreprocessor define to any class, struct or function that needs to be exposed publicly to third party software. Since the number of files in this library is not small, I've patched all what I found needed to compile the whole test suite. It would be good to use another third party software using FCL extensively to test that I did not forget about any of them.The PR also adds the
FCL_HIDE_ALL_SYMBOLSoption (OFF by the default). This option covers a use case we found in Drake where a software uses FCL (linked statically) but does not expose it in the public API (headers). In this scenario the best option to avoid any possible conflict with other versions of fcl is to hide all the symbols from the public ABI.Numbers before and after:
Summary: 13% symbols less and 6% size less .
The PR will alter the ABI so a new bump in the FCL version prior to release it would be ideal.