taocpp / taopq

C++ client library for PostgreSQL
Boost Software License 1.0
265 stars 40 forks source link

Shared library Windows build: exporting symbols, is it acceptable? #13

Closed emmenlau closed 3 years ago

emmenlau commented 6 years ago

Dear developers, thanks again for the very great work.

I have seen that the shared library build does not export any symbols on Microsoft Windows. With the help of cmake its nowadays quite easy to add explicit symbol visibility. When using explicit symbol visibility its also possible to default to hidden symbols on Un*x. Therefore the whole library becomes slightly more encapsulated.

I can provide such a pull request if you are interested? Please give me a heads up first, before I go through the effort to add the changes.

The change adds a new header postgres_export.h that is auto-generated by cmake. The header works in combination with an (automatic) cmake build-time definition that ensures that symbols become visible. In CMakeLists.txt, the required change is small:

include (GenerateExportHeader)
generate_export_header (postgres)
install (FILES ${CMAKE_CURRENT_BINARY_DIR}/postgres_export.h DESTINATION include)

The new header postgres_export.h needs to be included in all publicly visible source files. The header defines a macro POSTGRES_EXPORT that encapsulates all the "magic" required for symbol visibility on all platforms. An example for a fully visible class:

class POSTGRES_EXPORT connection final
         : public std::enable_shared_from_this< connection >

And a visible method:

POSTGRES_EXPORT std::string printf( const char* format, ... );

I can make all the required changes and provide a PR if you deem it interesting? Without such a change, a Windows shared library build can either only export all or no symbols.

d-frey commented 6 years ago

I'm interested in a PR, but I have a few reservations: CMake is currently not a requirement for our libraries, hence I'd prefer it if it would be possible to add an appropriately named header with an appropriately named macro. There surely shouldn't be too many variants required, maybe one for MSVC, one for GCC/Clang, and simply define the macro to nothing if the platform isn't detected?

emmenlau commented 6 years ago

Thanks for the nice answer! Yes, its certainly possible to use the auto-generated header as a fixed standard header (like any other). It is well developed and sufficient for all major platforms and compilers. The only thing to consider is the correct define that must be present at library build time and absent afterwards when using the library headers in other projects. I can add this to CMake but I'm not sure about your other build system(s)... it should be fairly easy though.

I got relatively far with a shared VS2017 build using the code in pr #14. However there are a few places where I would need help. After adding a number of visibility definitions, VS2017 still complains about missing symbols and I'm slightly lost why. As you can see in pr #14 the corresponding symbols should be exported, so I fail to see whats wrong. Does any of this ring a bell with you?

[...]
example.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class std::tuple<int,class std::experimental::optional<int>,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > __cdecl tao::postgres::result_traits<class std::tuple<int,class std::experimental::optional<int>,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > >,void>::from(class tao::postgres::row const &)" (__imp_?from@?$result_traits@V?$tuple@HV?$optional@H@experimental@std@@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@3@@std@@X@postgres@tao@@SA?AV?$tuple@HV?$optional@H@experimental@std@@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@3@@std@@AEBVrow@23@@Z) referenced in function "public: class std::tuple<int,class std::experimental::optional<int>,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > __cdecl tao::postgres::row::get<class std::tuple<int,class std::experimental::optional<int>,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > >(unsigned __int64)const " (??$get@V?$tuple@HV?$optional@H@experimental@std@@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@3@@std@@@row@postgres@tao@@QEBA?AV?$tuple@HV?$optional@H@experimental@std@@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@3@@std@@_K@Z)
src\test\postgres\tao-postgres-test-example.exe : fatal error LNK1120: 1 unresolved externals
result.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class std::experimental::optional<int> __cdecl tao::postgres::result_traits<class std::experimental::optional<int>,void>::null(void)" (__imp_?null@?$result_traits@V?$optional@H@experimental@std@@X@postgres@tao@@SA?AV?$optional@H@experimental@std@@XZ) referenced in function "public: class std::experimental::optional<int> __cdecl tao::postgres::row::get<class std::experimental::optional<int> >(unsigned __int64)const " (??$get@V?$optional@H@experimental@std@@@row@postgres@tao@@QEBA?AV?$optional@H@experimental@std@@_K@Z)
result.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class std::experimental::optional<int> __cdecl tao::postgres::result_traits<class std::experimental::optional<int>,void>::from(char const *)" (__imp_?from@?$result_traits@V?$optional@H@experimental@std@@X@postgres@tao@@SA?AV?$optional@H@experimental@std@@PEBD@Z) referenced in function "public: class std::experimental::optional<int> __cdecl tao::postgres::row::get<class std::experimental::optional<int> >(unsigned __int64)const " (??$get@V?$optional@H@experimental@std@@@row@postgres@tao@@QEBA?AV?$optional@H@experimental@std@@_K@Z)
result.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static struct std::pair<int,int> __cdecl tao::postgres::result_traits<struct std::pair<int,int>,void>::from(class tao::postgres::row const &)" (__imp_?from@?$result_traits@U?$pair@HH@std@@X@postgres@tao@@SA?AU?$pair@HH@std@@AEBVrow@23@@Z) referenced in function "public: struct std::pair<int,int> __cdecl tao::postgres::row::get<struct std::pair<int,int> >(unsigned __int64)const " (??$get@U?$pair@HH@std@@@row@postgres@tao@@QEBA?AU?$pair@HH@std@@_K@Z)
result.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class std::tuple<int,int,int,int> __cdecl tao::postgres::result_traits<class std::tuple<int,int,int,int>,void>::from(class tao::postgres::row const &)" (__imp_?from@?$result_traits@V?$tuple@HHHH@std@@X@postgres@tao@@SA?AV?$tuple@HHHH@std@@AEBVrow@23@@Z) referenced in function "public: class std::tuple<int,int,int,int> __cdecl tao::postgres::row::get<class std::tuple<int,int,int,int> >(unsigned __int64)const " (??$get@V?$tuple@HHHH@std@@@row@postgres@tao@@QEBA?AV?$tuple@HHHH@std@@_K@Z)
result.cpp.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static struct std::pair<int const ,int> __cdecl tao::postgres::result_traits<struct std::pair<int const ,int>,void>::from(class tao::postgres::row const &)" (__imp_?from@?$result_traits@U?$pair@$$CBHH@std@@X@postgres@tao@@SA?AU?$pair@$$CBHH@std@@AEBVrow@23@@Z) referenced in function "public: struct std::pair<int const ,int> __cdecl tao::postgres::row::get<struct std::pair<int const ,int> >(unsigned __int64)const " (??$get@U?$pair@$$CBHH@std@@@row@postgres@tao@@QEBA?AU?$pair@$$CBHH@std@@_K@Z)
src\test\postgres\tao-postgres-test-result.exe : fatal error LNK1120: 5 unresolved externals
d-frey commented 3 years ago

This issue and the PR for it (#14) both seem outdated and abandoned. I'm closing both for now, if you feel you want to give this another try I'd like to see some numbers/discussion on actual benefits, and after that a new PR based on the current code. Thanks for your effort so far!