-
Notifications
You must be signed in to change notification settings - Fork 61
Add visibility feature #331
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
Conversation
a1ea3c5 to
40caf3a
Compare
| import feature ; | ||
|
|
||
| feature.feature visibility | ||
| : hidden protected global |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the default to be different than the for compilers will affect everyone, not jus boost builds. Please change the default to match the compiler defaults. If boost wants a different kind of build it can set the feature in the appropriate jamfiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, this needs to be an optional feature so that the compile option doesn't appear when not specified. I.e. which should be the default.
40caf3a to
4d81d63
Compare
| import feature ; | ||
|
|
||
| feature.feature visibility | ||
| : hidden protected global |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, this needs to be an optional feature so that the compile option doesn't appear when not specified. I.e. which should be the default.
|
|
||
| feature.feature visibility | ||
| : global protected hidden | ||
| : propagated ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, this needs to be an optional feature so that the compile option doesn't appear when not specified. I.e. which should be the default.
The new visibility feature can be used to specify default symbol visibility on compilers and platforms that support it. The default visibility is global, which matches most compilers' defaults. In gcc documentation it is called the "default" visibility. Other modes are: protected and hidden.
4d81d63 to
9058b34
Compare
|
I've updated the feature and the docs according the comments, although it sort of defeats the purpose. We could modify library jamfiles before, the point was to make hidden visibility the default so that we don't have to do that. At least the property offers a more portable way to do that. |
|
AMDG
On 08/19/2018 11:47 AM, Andrey Semashev wrote:
I've updated the feature and the docs according the comments, although it sort of defeats the purpose. We could modify library jamfiles before, the point was to make hidden visibility the default so that we don't have to do that. At least the property offers a more portable way to do that.
It can be set in the Boost Jamroot:
project /boost : ... : default-build <visibility>hidden ;
In Christ,
Steven Watanabe
|
|
Thanks, Steven, that sounds like a good idea. I'll create a PR for the superproject if this PR is accepted. |
|
The related superproject PR is boostorg/boost#190. |
|
Please merge to master. Used that feature in tests, now they fail on master (but do work perfectly on develop) |
|
Already done in aa73dbc |
The new visibility feature can be used to specify default symbol visibility
on compilers and platforms that support it. The default visibility is
hidden, unlike most compilers' defaults. This is intentional so that all
Boost libraries are compiled with hidden visibility by default. Other modes
are: protected and global (the latter is called "default" in gcc documentation).
I've only tested it on Linux, with gcc and clang.
The discussion that led to this PR is here:
http://boost.2283326.n4.nabble.com/all-Request-for-out-of-the-box-visibility-support-tt4704134.html