jll63 / yomm2

Fast, orthogonal, open multi-methods. Solve the Expression Problem in C++17.
Boost Software License 1.0
343 stars 18 forks source link

Issues with Debug build of depending library (due to recent changes) #17

Closed derpda closed 3 years ago

derpda commented 3 years ago

Problem

Our library, here called example, uses YOMM2. Building example in Release works fine, but when building example in Debug configuration we get the following error during linking:

/usr/bin/ld: ../../lib/libexample.so: undefined reference to `yorel::yomm2::detail::log()'

Explanation

This problem is introduced in PR #16 since it will now only define yomm2::detail::log() when YOMM2_ENABLE_TRACE is true. YOMM2_ENABLE_TRACE is set to true depending on the NDEBUG flag in the yomm2.hpp header, meaning that it actually depends on the including libraries compilation settings (which is fine I guess). The issue is using this flag in the compiled yomm2.cpp in a way that breaks this dependency.

16 causes the problem by encapsulating the entire definition of log() inside an if statement depending on the value of YOMM2_ENABLE_TRACE at the time of compiling YOMM2.

However, log() is still used depending on the value of YOMM2_ENABLE_TRACE at the time of compiling the depending library.

Suggestion

The goal is to meet the requirement in #15 (which is fixed by #16) of being able to compile with target_compile_definitions(yomm2 PUBLIC YOMM2_ENABLE_TRACE=0) in the top-level CMakeLists.txt, thus the added #include <sstream> needs to stay. However, the check on YOMM2_ENABLE_TRACE around the log() function are not necessary, and the definition of a simple function is no significant burden on the (either way short) compilation. Thus, my suggestion is to remove these checks.

jll63 commented 3 years ago

Thanks. I was wondering why the mixed build tests did not catch this. It is because of #12. I put a simple fix in #19. Can you review it? Your cmake is better than mine...