Add support for -fvisibility=hidden and -fvisibility=default flags#415
Add support for -fvisibility=hidden and -fvisibility=default flags#415csabaszigetipix4d wants to merge 3 commits intodropbox:masterfrom
-fvisibility=hidden and -fvisibility=default flags#415Conversation
…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: |
artwyman
left a comment
There was a problem hiding this comment.
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.
|
|
||
| set(SRC_SHARED | ||
| "support-lib/djinni_common.hpp" | ||
| "support-lib/project_export.hpp" |
There was a problem hiding this comment.
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.
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=hiddenand-fvisibility=defaultDjinni supports both the
-fvisibility=hiddenand the-fvisibility=defaultflags.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_EXPORTis 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.hppheader.This
PROJECT_EXPORTdoes nothing in case if you are using the-fvisibility=defaultcompiler flag.This
PROJECT_EXPORTin case if you are using the-fvisibility=defaultcompiler 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_DLLbased 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.hppheader.