bluescarni / obake

A C++20 library for the symbolic manipulation of sparse polynomials & co.
https://bluescarni.github.io/obake
Mozilla Public License 2.0
22 stars 8 forks source link

Visibility linker warnings #147

Closed andrewcorrigan closed 1 year ago

andrewcorrigan commented 1 year ago

I'm sorry I missed this in #146. But I started getting warnings like:

ld: warning: direct access in function 'obake::detail::ss_intersect_idx(boost::container::flat_set<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, void> const&, boost::container::flat_set<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, void> const&)' from file '/obake/Debug/libobake.a(symbols.o)' to global weak symbol 'obake::safe_cast<unsigned long>' from file '/Debug/Objects-normal/x86_64/test.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

These go away by also guarding the properties with OBAKE_BUILD_STATIC_LIBRARY:

if(OBAKE_BUILD_STATIC_LIBRARY)
    add_library(obake STATIC "${OBAKE_SRC_FILES}")
    target_compile_definitions(obake PUBLIC OBAKE_STATIC_BUILD)
else()
    add_library(obake SHARED "${OBAKE_SRC_FILES}")
    set_property(TARGET obake PROPERTY VERSION "8.0")
    set_property(TARGET obake PROPERTY SOVERSION 8)
    set_target_properties(obake PROPERTIES CXX_VISIBILITY_PRESET hidden)
    set_target_properties(obake PROPERTIES VISIBILITY_INLINES_HIDDEN TRUE)
endif()

This suppresses the warnings, but I don't really understand the visibility properties, so am wondering does this change make sense?

bluescarni commented 1 year ago

@andrewcorrigan yes indeed, the visibility settings need to be set up when building in dynamic library mode. I'll fix this.

bluescarni commented 1 year ago

This should now be fixed in #149, closing.