-
Notifications
You must be signed in to change notification settings - Fork 488
Add support for -fvisibility=hidden
and -fvisibility=default
flags
#415
base: master
Are you sure you want to change the base?
Add support for -fvisibility=hidden
and -fvisibility=default
flags
#415
Conversation
…ject_export.hpp 1.: Added new Djinni support-lib header : project_export.hpp 2.: Moved PROJECT_EXPORT definition from djinni_common.hpp to the new project_export.hpp file 3.: Remove every " // needed for PROJECT_EXPORT" comments 4.: Updated README 5.: Fixed automated tests of Djinni
Hi, @csabaszigetipix4d: |
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.
I've only taken a high-level look so far. I think the idea of making visibility controllable is a good one, but it should probably be configurable rather than having a single default behavior. As I mention in one of my inline comments, I don't think that exporting these specific symbols is always what's right for all use cases.
@@ -6,6 +6,7 @@ include(GNUInstallDirs) | |||
|
|||
set(SRC_SHARED | |||
"support-lib/djinni_common.hpp" | |||
"support-lib/project_export.hpp" |
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.
This is used so widely (requiring it to be in the include path) that it really needs a more unique name, like djinni_project_export.hpp
.
|
||
You can pass `-fvisibility=hidden` or the `-fvisibility=default` flags to your compiler, Djinni handles both cases well. | ||
|
||
The symbols that are belonging to the public interfaces that are generated by Djinni are automatically defined to be visible symbols. |
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.
I feel like it's not obvious that making these symbols always public is the right behavior for all users. E.g. if Djinni symbols are only used internally to a larger library, they wouldn't need to be exported. In the use case we've used at Dropbox, we build all our C++ code into a single library on Android, so only the JNI functions need to be exported.
I quote the updated README.md here :
Support for
-fvisibility=hidden
and-fvisibility=default
Djinni supports both the
-fvisibility=hidden
and the-fvisibility=default
flags.The symbols that are belonging to the public interfaces that are generated by Djinni are automatically defined to be visible symbols.
To achieve this Djinni is marking the generated code with
PROJECT_EXPORT
.PROJECT_EXPORT
is defined insupport-lib/djinni_common.hpp
.This macro is using the correct attribute specifier for the actual compiler. For more details, please check the macro definition in the
support-lib/djinni_common.hpp
header.This
PROJECT_EXPORT
does nothing in case if you are using the-fvisibility=default
compiler flag.This
PROJECT_EXPORT
in case if you are using the-fvisibility=default
compiler flag.Since the generated headers are including this
support-lib/djinni_common.hpp
, you must distribute it together with your library that is using Djinni.Windows
For Windows builds, you must define
BUILDING_DLL
based on your needs.You can define it as a compiler option like
-DBUILDING_DLL
.For more details, please check the macro definition in the
support-lib/djinni_common.hpp
header.