Get CMake to treat JUCE headers as system headers

[bepzi]

I can't enable any compiler warning flags because it slows down the build when analyzing the JUCE code, and triggers warnings that are out of our control.

I'd like it if the compiler could be informed that the JUCE code is not our code, and therefore, that it doesn't need to check it or report warnings for it. This would also speed up the clang-tidy lint step.

[GavinRay97]

Maybe you can use -isystem, and I think CMake has the SYSTEM property on functions using "includes" in the name for emulating this.

In clang there is also --system-header-prefix: https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers

The –system-header-prefix= and –no-system-header-prefix= command-line arguments can be used to override whether subsets of an include path are treated as system headers. When the name in a #include directive is found within a header search path and starts with a system prefix, the header is treated as a system header. The last prefix on the command-line which matches the specified header name takes precedence. For instance:

$ clang -Ifoo -isystem bar --system-header-prefix=x/ \
    --no-system-header-prefix=x/y/

# Brute force it, unsure if this will work but you can try
set(CMAKE_CXX_FLAGS "-isystem'./third-party/JUCE/modules' ${CMAKE_CXX_FLAGS}")

# Modified `target_link_libraries()` that specifies SYSTEM visibility to ignore warnings
# https://stackoverflow.com/a/52136398
function(target_link_libraries_system target)
  set(options PRIVATE PUBLIC INTERFACE)
  cmake_parse_arguments(TLLS "${options}" "" "" ${ARGN})
  foreach(op ${options})
    if(TLLS_${op})
      set(scope ${op})
    endif()
  endforeach(op)
  set(libs ${TLLS_UNPARSED_ARGUMENTS})

  foreach(lib ${libs})
    get_target_property(lib_include_dirs ${lib} INTERFACE_INCLUDE_DIRECTORIES)
    if(lib_include_dirs)
      if(scope)
        target_include_directories(${target} SYSTEM ${scope} ${lib_include_dirs})
      else()
        target_include_directories(${target} SYSTEM PRIVATE ${lib_include_dirs})
      endif()
    else()
      message("Warning: ${lib} doesn't set INTERFACE_INCLUDE_DIRECTORIES. No include_directories set.")
    endif()
    if(scope)
      target_link_libraries(${target} ${scope} ${lib})
    else()
      target_link_libraries(${target} ${lib})
    endif()
  endforeach()
endfunction(target_link_libraries_system)

target_link_libraries_system(Elysium
  PRIVATE
  juce::juce_audio_basics
  # ...

  elysium_rust

  PUBLIC
  juce::juce_recommended_config_flags
  # ...
  )

Btw, if you are using a clangd based Language Server -- since LLVM 12, clangd includes clang-tidy binary in it and allows you to specify the clang-tidy config, but also the compile configs.

I use the below in JUCE to have clangd be able to parse the majority of the classes (they use JUCE_API which is not defined in-scope and my compile_commands.json doesn't include a definition). .clangd has a much easier syntax for specifying checks and options =D

CompileFlags: # Tweak the parse settings
  Add:
    - "-xc++"
    - "-std=c++20"
    - "-DJUCE_API=__declspec(dllexport)" # This just exists so every single class with JUCE_API doesn't break syntactically with clangd LSP

Index:
  Background: Build # Build a type + incoming reference index in the background

Diagnostics:
  # https://clang.llvm.org/extra/clang-tidy/
  ClangTidy:
    Add:
      - bugprone* # Checks that target bugprone code constructs.
      - clang-analyze* # Clang Static Analyzer checks.
      - concurrency* # Checks related to concurrent programming (including threads, fibers, coroutines, etc.).
      # - cppcoreguidelines* # Checks related to C++ Core Guidelines.
      - modernize* # Checks that advocate usage of modern (currently “modern” means “C++11”) language constructs.
      - performance* # Checks that target performance-related issues.
      - portability* # Checks that target portability-related issues that don’t relate to any particular coding style.
      - readability* # Checks that target readability-related issues that don’t relate to any particular coding style.
    # Add: modernize*
    # Remove: modernize-use-trailing-return-type
    # CheckOptions:
      # readability-identifier-naming.VariableCase: CamelCase

[bepzi]

Hi! Thank you for taking the time to write this up, I really appreciate the research you've done on this.

In clang there is also --system-header-prefix: https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers

That does sound interesting, although I'd be hesitant to make this the primary solution as I'd like to maintain at least some compatibility with GCC and MSVC. I am starting to realize though that if you want to do anything fancy, like cross-language LTO, you pretty much have to use Clang anyways, but I like how I can just cmake --build on a Windows machine and get a working binary with vanilla MSVC.

function(target_link_libraries_system target)
 set(options PRIVATE PUBLIC INTERFACE)
 cmake_parse_arguments(TLLS "${options}" "" "" ${ARGN})
 foreach(op ${options})
   if(TLLS_${op})
     set(scope ${op})
   endif()
 endforeach(op)
 set(libs ${TLLS_UNPARSED_ARGUMENTS})

This is some advanced CMake :sweat_smile: I'll give it a try and see what happens.

Btw, if you are using a clangd based Language Server -- since LLVM 12, clangd includes clang-tidy binary in it and allows you to specify the clang-tidy config, but also the compile configs.

That is interesting, I'll have to get this set up. Right now I haven't really set up my C++ language server stuff because I don't need to spend much time in the C++ parts of the codebase anyways (hooray!) I think I was using ccls before, but if clangd is better these days I may switch over to it.

[bepzi]

Update: I gave the target_link_libraries_system function a try, and unfortunately it doesn't work because JUCE thought they'd be clever and transitively add their source code to your target whenever you link to them. So the target_link_libraries call inside the target_link_libraries_system adds their sources to your library, and even though we're properly turning the headers into SYSTEM headers, we still compile their code with our warning flags.

I did find a hacky workaround that seems to work, though. It turns out it's possible to specify per-file compiler options that take precedence over global and target compiler options, so if I make my target compile without compiler warnings, and then tell CMake to compile just my source files with compiler warnings (as though they're not part of my target), it successfully avoids checking the JUCE code.

[GavinRay97]

Would you be willing to post that CMake code out of curiosity? Sounds useful

[bepzi]

Would you be willing to post that CMake code out of curiosity? Sounds useful

Yeah, it's pretty straightforward. Basically, don't define any compiler flags on your main target; instead, define it on the specific source files you want to check:

set_source_files_properties(${all_the_sources} PROPERTIES COMPILE_OPTIONS
    -Wall -Wextra # etc....)

I'll have the real thing in this repo eventually, I just want to make sure I do it in a maintainable way that's compatible with multiple compilers.

It's a hack, but it might be the best we can do when JUCE is stapling their sources onto our target.