remymuller / juce-cmake

CMake find module for the JUCE library
MIT License
30 stars 7 forks source link

Juce source files are compiled with the same warning flags as the target Juce is linked to #20

Open TobiasLudwig opened 4 years ago

TobiasLudwig commented 4 years ago

When linking ${JUCE_LIBRARIES} to a target, the corresponding Juce source files are compiled with the same warning flags that are specifically set to the target. This may result in warnings in Juce code which renders the warning flags useless for the target.

This example triggers C4388 / -Wold-style-cast warnings in juce_core when compiling with MSVC / clang-cl. CMakeLists.txt

project(JuceCmake CXX)

list(APPEND CMAKE_MODULE_PATH "PATH/TO/juce-cmake/cmake")
set(JUCE_ROOT "PATH/TO/JUCE/")
find_package(JUCE REQUIRED
    COMPONENTS
    juce_core
)

add_executable(
    minimal_example
    "main.cpp"         # content: "int main() { return 0; }"
)
if (${CMAKE_CXX_COMPILER_ID} STREQUAL "MSVC")
    set(warning_flags "/w14388") # '==' signed/unsigned mismatch
else()
    set(warning_flags "-Wold-style-cast")
endif()
target_compile_options(
    minimal_example
    PRIVATE
    ${warning_flags}
)

target_link_libraries(
    minimal_example
    ${JUCE_LIBRARIES}
)

Expected behavior: The Juce source files are compiled with their own set of warning flags that don't interfere with the target Juce is linked to.

Disabling warnings for ${JUCE_LIBRARIES} doesn't work, as this is an interface library and the warnings would get disabled for the target too:

target_compile_options(
    ${JUCE_LIBRARIES}
    INTERFACE
    "/wd4388"
)
remymuller commented 4 years ago

This is a direct consequence of the fact that by design, in order to match the way juce projects are created by the Projucer, juce code compilation is postponed to the target it is linked against.

A way to get around this, could be to create an intermediate static library linking against ${JUCE_LIBRARIES} to collect all JUCE code (cmake may require using a dummy C++ file for that) and then link your executable against this static library.

TobiasLudwig commented 4 years ago

I also thought about adding an intermediate static library, but how well does this play with juce_add_audio_plugin? Unfortunately I'm not that familiar with the Juce build process. Is there a reason for not building each module or ${JUCE_LIBRARIES} as a static library (e.g. unreferenced global variables that would be removed) or is this only done for replicating the Projucer as close as possible?

Something completely different: I noticed that you don't add "-DJUCE_APP_CONFIG_HEADER=\"${JUCE_APPCONFIG_H}\"" to ${JUCE_LIBRARIES} but instead include AppConfig.h in every generated include_juce_*.(cpp|mm) file. Each Juce header includes (transitively) juce_core.h which includes juce_core/system/juce_TargetPlatform.h which then includes JUCE_APP_CONFIG_HEADER. So adding this define would remove the need for including AppConfig.h before any Juce file is included.