sakra / cotire

CMake module to speed up builds.
MIT License
1.3k stars 143 forks source link

CXX_COMPILER_ID generator expression in compile definitions #135

Open JoshCollins746 opened 7 years ago

JoshCollins746 commented 7 years ago

I am trying to use the GSL (https://github.com/Microsoft/GSL) with cotire in a project. GSL adds compile definitions to the INTERFACE target using generator expressions:

target_compile_definitions(GSL INTERFACE
    $<$<CXX_COMPILER_ID:MSVC>:
        # remove unnecessary warnings about unchecked iterators
        _SCL_SECURE_NO_WARNINGS
    >
)

When I run cotire 1.7.10 on my target after linking to this library I get the following error:

CMake Error at cotire/CMake/cotire.cmake:2254 (file):
  Error evaluating generator expression:

    $<CXX_COMPILER_ID:MSVC>

  $<CXX_COMPILER_ID> may only be used with binary targets.  It may not be
  used with add_custom_command or add_custom_target.
Call Stack (most recent call first):
  cotire/CMake/cotire.cmake:2848 (cotire_generate_target_script)
  cotire/CMake/cotire.cmake:3254 (cotire_process_target_language)
  cotire/CMake/cotire.cmake:3431 (cotire_target)
  CMakeLists.txt:16 (cotire)

It seems that cotire does not like these generator expressions. A minimal reproduction is:

cmake_minimum_required(VERSION 3.7)
project(Test CXX)

set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cotire/CMake")
include(cotire)

add_executable(Test main.cpp)

target_compile_definitions(Test PUBLIC $<$<CXX_COMPILER_ID:MSVC>:_SCL_SECURE_NO_WARNINGS>)

cotire(Test)

I have tested with both cmake 3.7 and 3.9.

I tried running cotire before adding the target_compile_options which removes the error, but I get the impression that this means that the definition is being ignored by cotire which seems would cause other issues.

I do not know if this is an error with Cotire or with the way that I am using it.

promgamer commented 6 years ago

Same problem here. Any solution?

JoshCollins746 commented 6 years ago

Hi @promgamer, unfortunately I have not resolved the core issue. My work around has been to patch the GSL cmake configuration to use an if block instead of generator expressions to apply the compile definitions for MSVC.

Quincunx271 commented 6 years ago

I hacked around this by adding the following code. Basically, I replace all instances of $<CXX_COMPILER_ID:CompilerId> with 1 or 0 depending on whether that compiler is the correct one:

# Replace CXX_COMPILER_ID, as cmake doesn't support that in this position
file (READ "${_targetCotireScript}" _targetCotireScriptFileContents)
string (REGEX MATCHALL "\\$<CXX_COMPILER_ID:([a-zA-Z0-9]*)>" _compilerIds "${_targetCotireScriptFileContents}")
set(_hasCompilerIds "")
foreach (compilerIdStr ${_compilerIds})
    string (REGEX REPLACE "\\$<CXX_COMPILER_ID:([a-zA-Z0-9]*)>" "\\1" _onlyCompilerId "${compilerIdStr}")
    if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "${_onlyCompilerId}")
        list(APPEND _hasCompilerIds "1")
    else()
        list(APPEND _hasCompilerIds "0")
    endif()
endforeach()
foreach (compilerIdReplacement ${_hasCompilerIds})
    list(GET _compilerIds 0 _compilerIdMatch)
    list(REMOVE_AT _compilerIds 0)
    string (REPLACE "${_compilerIdMatch}" "${compilerIdReplacement}" _targetCotireScriptFileContents "${_targetCotireScriptFileContents}")
endforeach()
file (WRITE "${_targetCotireScript}" "${_targetCotireScriptFileContents}")

Here's a link to a patch that adds that.

sakra commented 6 years ago

cotire uses the CMake command file(GENERATE ... to expand generator expressions used in various compilation settings. This command does not handle certain CMake generator expressions like $<CXX_COMPILER_ID:...> correctly.

promgamer commented 6 years ago

Is there something we can do to fix this?

sakra commented 6 years ago

Currently the only option is a patch along the lines of Quincunx271's patch above. This will however not handle deeply nested CXX_COMPILER_ID generator expressions.

Quincunx271 commented 6 years ago

Perhaps we could ask if this feature could be added, on CMake's repository. We might have to be the ones to patch CMake's code, though, and they might not accept the feature request.

Naios commented 6 years ago

I've opened the PR #155 targeting this issue which rewrites the affected generator expressions to a STREQUAL generator expression with the global value expanded. This should also handle deeply nested CXX_COMPILER_ID generator expressions. It would be great if you could test the PR so we can verify that it fixes this issue. Affected expressions can be added in the future through expanding the foreach.