martin-olivier / dylib

C++ cross-platform wrapper around dynamic loading of shared libraries (dll, so, dylib)
https://conan.io/center/recipes/dylib
MIT License
293 stars 44 forks source link

DYLIB_API questionable #36

Closed eyalroz closed 2 years ago

eyalroz commented 2 years ago

dylib is not supposed to have any requirements from dynamic-linking libraries. They exist entirely independently of it, ignorant of its existence. So, where would they know about DYLIB_API from? ... nowhere.

As for the code linking against them, using dylib - it shouldn't have to know about that either; it just defines a function type, e.g.int(void), and get_function()'s with that.

So - it seems to me like this:

#if defined(_WIN32) || defined(_WIN64)
#define DYLIB_API extern "C" __declspec(dllexport)
#else
#define DYLIB_API extern "C"
#endif

is unnecessary. Perhaps it belongs in the test library?

martin-olivier commented 2 years ago

I'm currently working on https://github.com/martin-olivier/dylib/issues/27 to be able to load C++ mangled symbols, and as you said DYLIB_API will become quite useless.

Here is the current changes on this on my local work :

#if defined(_WIN32) || defined(_WIN64)
#define DYLIB_EXPORT __declspec(dllexport)
#else
#define DYLIB_EXPORT
#endif

The user will be able to totally get rid of DYLIB_EXPORT by using the following cmake rule : (I will add this in the documentation)

if(MSVC)
    set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
endif()
eyalroz commented 2 years ago

I'm saying that the user can't use DYLIB_API anyway, already today. The user is not the person writing the dynamic library...

martin-olivier commented 2 years ago

Mhhh yes I see your point. Originally, I made this library to facilitate my school projects. Since we were providing both core and dlls, I used it to export/unmangle my factory functions.

But yes, dlls shouldn't include dylib just for that macro and in most cases dlls are shipped by other people than the one writing the core.

I will remove DYLIB_API / DYLIB_EXPORT