From 8f63f3b04ec36b8adc8bcdef38204cb073131229 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 4 Jun 2021 14:47:52 -0400 Subject: [PATCH 1/7] cmVisualStudio10TargetGenerator: remove unused variable --- Source/cmVisualStudio10TargetGenerator.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/cmVisualStudio10TargetGenerator.cxx b/Source/cmVisualStudio10TargetGenerator.cxx index b79c6fda14..822abb3788 100644 --- a/Source/cmVisualStudio10TargetGenerator.cxx +++ b/Source/cmVisualStudio10TargetGenerator.cxx @@ -3543,8 +3543,6 @@ void cmVisualStudio10TargetGenerator::WriteNasmOptions( } Elem e2(e1, "NASM"); - std::vector includes = - this->GetIncludes(configName, "ASM_NASM"); OptionsHelper nasmOptions(*(this->NasmOptions[configName]), e2); nasmOptions.OutputAdditionalIncludeDirectories("ASM_NASM"); nasmOptions.OutputFlagMap(); From b094324948291ee3fbd2ce17969157cdeab18592 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 4 Jun 2021 10:56:43 -0400 Subject: [PATCH 2/7] Tests/IncludeDirectories: Include system headers via angle brackets This is typically how projects include them, and cl's `-external:{I,W}` flags suppress warnings only when included through angle brackets. --- Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp | 3 ++- .../SystemIncludeDirectories/imported_consumer.cpp | 2 +- Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp b/Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp index a13f08fdc0..3da308d5d8 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp @@ -1,5 +1,6 @@ -#include "config_iface.h" +#include + #include "upstream.h" int consumer() diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/imported_consumer.cpp b/Tests/IncludeDirectories/SystemIncludeDirectories/imported_consumer.cpp index 1dbe819e13..53759b11c5 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/imported_consumer.cpp +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/imported_consumer.cpp @@ -1,5 +1,5 @@ -#include "systemlib.h" +#include int main() { diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h b/Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h index a670c2a48d..3daf69eb6c 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h @@ -2,7 +2,7 @@ #ifndef UPSTREAM_H #define UPSTREAM_H -#include "systemlib.h" +#include #ifdef _WIN32 __declspec(dllexport) From 809f7b0c3af8d46610ae0963e061415b50f2a363 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 2 Jun 2021 19:20:25 -0400 Subject: [PATCH 3/7] Tests/IncludeDirectories: fix copy pasta for otherlib --- .../IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt index dee39c8264..ff52c985d9 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt @@ -52,7 +52,7 @@ target_compile_options(somelib PRIVATE -Werror=unused-variable) # add a target which consumes a relative system include add_library(otherlib upstream.cpp) target_link_libraries(otherlib PUBLIC somelib) -target_compile_options(somelib PRIVATE -Werror=unused-variable) +target_compile_options(otherlib PRIVATE -Werror=unused-variable) macro(do_try_compile error_option) set(TC_ARGS From 20ab49193ba1196421909b7910c9e0c7a30fa31f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 2 Jun 2021 19:19:45 -0400 Subject: [PATCH 4/7] Tests/IncludeDirectories: factor out applying flags to targets --- .../SystemIncludeDirectories/CMakeLists.txt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt index ff52c985d9..7874d5e03c 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt @@ -6,9 +6,13 @@ project(SystemIncludeDirectories) add_library(systemlib systemlib.cpp) target_include_directories(systemlib PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/systemlib") +function (apply_error_flags target) + target_compile_options(${target} PRIVATE -Werror=unused-variable) +endfunction () + add_library(upstream upstream.cpp) target_link_libraries(upstream LINK_PUBLIC systemlib) -target_compile_options(upstream PRIVATE -Werror=unused-variable) +apply_error_flags(upstream) target_include_directories(upstream SYSTEM PUBLIC $ @@ -29,7 +33,7 @@ endif() add_library(consumer consumer.cpp) target_link_libraries(consumer upstream config_specific) -target_compile_options(consumer PRIVATE -Werror=unused-variable) +apply_error_flags(consumer) add_library(iface IMPORTED INTERFACE) set_property(TARGET iface PROPERTY INTERFACE_INCLUDE_DIRECTORIES @@ -38,21 +42,21 @@ set_property(TARGET iface PROPERTY INTERFACE_INCLUDE_DIRECTORIES add_library(imported_consumer imported_consumer.cpp) target_link_libraries(imported_consumer iface) -target_compile_options(imported_consumer PRIVATE -Werror=unused-variable) +apply_error_flags(imported_consumer) add_library(imported_consumer2 imported_consumer.cpp) target_link_libraries(imported_consumer2 imported_consumer) -target_compile_options(imported_consumer2 PRIVATE -Werror=unused-variable) +apply_error_flags(imported_consumer2) # add a target which has a relative system include add_library(somelib imported_consumer.cpp) target_include_directories(somelib SYSTEM PUBLIC "systemlib_header_only") -target_compile_options(somelib PRIVATE -Werror=unused-variable) +apply_error_flags(somelib) # add a target which consumes a relative system include add_library(otherlib upstream.cpp) target_link_libraries(otherlib PUBLIC somelib) -target_compile_options(otherlib PRIVATE -Werror=unused-variable) +apply_error_flags(otherlib) macro(do_try_compile error_option) set(TC_ARGS From 399a3204bb2d198cc660cd83e0de39ad95658498 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 2 Jun 2021 19:21:38 -0400 Subject: [PATCH 5/7] Tests/IncludeDirectories: align sibling predicates --- Tests/IncludeDirectories/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/IncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/CMakeLists.txt index d4c19c79b5..0f0f1395fa 100644 --- a/Tests/IncludeDirectories/CMakeLists.txt +++ b/Tests/IncludeDirectories/CMakeLists.txt @@ -2,7 +2,8 @@ cmake_minimum_required (VERSION 2.6) project(IncludeDirectories) if (((CMAKE_C_COMPILER_ID STREQUAL GNU AND CMAKE_C_COMPILER_VERSION VERSION_GREATER 4.4) - OR (CMAKE_C_COMPILER_ID STREQUAL Clang AND NOT "x${CMAKE_CXX_SIMULATE_ID}" STREQUAL "xMSVC") OR CMAKE_C_COMPILER_ID STREQUAL AppleClang) + OR (CMAKE_C_COMPILER_ID STREQUAL Clang AND NOT "x${CMAKE_CXX_SIMULATE_ID}" STREQUAL "xMSVC") + OR CMAKE_C_COMPILER_ID STREQUAL AppleClang) AND (CMAKE_GENERATOR STREQUAL "Unix Makefiles" OR CMAKE_GENERATOR STREQUAL "Ninja" OR (CMAKE_GENERATOR STREQUAL "Xcode" AND NOT XCODE_VERSION VERSION_LESS 6.0))) From 5a5c85dffd72972987cb542a4b6c9e606920cbb5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 2 Jun 2021 19:22:35 -0400 Subject: [PATCH 6/7] Tests/IncludeDirectories: support MSVC in system include tests --- Tests/IncludeDirectories/CMakeLists.txt | 23 ++++++++++++------- .../SystemIncludeDirectories/CMakeLists.txt | 12 ++++++++-- .../CMakeLists.txt | 4 ++-- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/Tests/IncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/CMakeLists.txt index 0f0f1395fa..4c488e6159 100644 --- a/Tests/IncludeDirectories/CMakeLists.txt +++ b/Tests/IncludeDirectories/CMakeLists.txt @@ -3,17 +3,24 @@ project(IncludeDirectories) if (((CMAKE_C_COMPILER_ID STREQUAL GNU AND CMAKE_C_COMPILER_VERSION VERSION_GREATER 4.4) OR (CMAKE_C_COMPILER_ID STREQUAL Clang AND NOT "x${CMAKE_CXX_SIMULATE_ID}" STREQUAL "xMSVC") - OR CMAKE_C_COMPILER_ID STREQUAL AppleClang) + OR CMAKE_C_COMPILER_ID STREQUAL AppleClang + OR ("x${CMAKE_C_COMPILER_ID}" STREQUAL "xMSVC" AND + CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL "19.29.30036.3" AND + NOT CMAKE_GENERATOR MATCHES "Visual Studio")) # No support for VS generators yet. AND (CMAKE_GENERATOR STREQUAL "Unix Makefiles" OR CMAKE_GENERATOR STREQUAL "Ninja" OR (CMAKE_GENERATOR STREQUAL "Xcode" AND NOT XCODE_VERSION VERSION_LESS 6.0))) - include(CheckCXXCompilerFlag) - check_cxx_compiler_flag(-Wunused-variable run_sys_includes_test) - if(run_sys_includes_test) - # The Bullseye wrapper appears to break the -isystem effect. - execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version OUTPUT_VARIABLE out ERROR_VARIABLE out) - if("x${out}" MATCHES "Bullseye") - set(run_sys_includes_test 0) + if ("x${CMAKE_C_COMPILER_ID}" STREQUAL "xMSVC") + set(run_sys_includes_test 1) + else () + include(CheckCXXCompilerFlag) + check_cxx_compiler_flag(-Wunused-variable run_sys_includes_test) + if(run_sys_includes_test) + # The Bullseye wrapper appears to break the -isystem effect. + execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version OUTPUT_VARIABLE out ERROR_VARIABLE out) + if("x${out}" MATCHES "Bullseye") + set(run_sys_includes_test 0) + endif() endif() endif() if (run_sys_includes_test) diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt index 7874d5e03c..a746a681b0 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt +++ b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt @@ -7,7 +7,11 @@ add_library(systemlib systemlib.cpp) target_include_directories(systemlib PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/systemlib") function (apply_error_flags target) - target_compile_options(${target} PRIVATE -Werror=unused-variable) + if (MSVC) + target_compile_options(${target} PRIVATE /we4101) + else () + target_compile_options(${target} PRIVATE -Werror=unused-variable) + endif () endfunction () add_library(upstream upstream.cpp) @@ -65,7 +69,11 @@ macro(do_try_compile error_option) LINK_LIBRARIES iface ) if (${error_option} STREQUAL WITH_ERROR) - list(APPEND TC_ARGS COMPILE_DEFINITIONS -Werror=unused-variable) + if (MSVC) + list(APPEND TC_ARGS COMPILE_DEFINITIONS /we4101) + else () + list(APPEND TC_ARGS COMPILE_DEFINITIONS -Werror=unused-variable) + endif () endif() try_compile(${TC_ARGS}) endmacro() diff --git a/Tests/IncludeDirectories/SystemIncludeDirectoriesPerLang/CMakeLists.txt b/Tests/IncludeDirectories/SystemIncludeDirectoriesPerLang/CMakeLists.txt index 70dfa017bd..5d58633e59 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectoriesPerLang/CMakeLists.txt +++ b/Tests/IncludeDirectories/SystemIncludeDirectoriesPerLang/CMakeLists.txt @@ -7,14 +7,14 @@ set_target_properties(c_interface PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "$<$:${CMAKE_CURRENT_SOURCE_DIR}>" INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "$<$:${CMAKE_CURRENT_SOURCE_DIR}>" ) -target_compile_options(c_interface INTERFACE "$<$:-Werror=unused-variable>") +target_compile_options(c_interface INTERFACE "$<$:-Werror=unused-variable>;$<$:/we4101>") add_library(cxx_interface INTERFACE) set_target_properties(cxx_interface PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "$<$:${CMAKE_CURRENT_SOURCE_DIR}/cxx_system_include>" INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "$<$:${CMAKE_CURRENT_SOURCE_DIR}/cxx_system_include>" ) -target_compile_options(cxx_interface INTERFACE "$<$:-Werror=unused-variable>") +target_compile_options(cxx_interface INTERFACE "$<$:-Werror=unused-variable>;$<$:/we4101>") # The C header must come before the C++ header for this test to smoke out the # failure. The order of sources is how CMake determines the include cache From f29e1874adbc31c9d4643816d8b520ad4fde3b84 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 19 May 2020 10:28:26 -0400 Subject: [PATCH 7/7] Compiler/MSVC: use the `-external:I` flag for system includes See: #17904 --- Help/release/dev/msvc-isystem.rst | 7 +++++++ Modules/Compiler/MSVC-C.cmake | 6 ++++++ Modules/Compiler/MSVC-CXX.cmake | 6 ++++++ Source/cmLocalGenerator.cxx | 8 ++++++++ 4 files changed, 27 insertions(+) create mode 100644 Help/release/dev/msvc-isystem.rst diff --git a/Help/release/dev/msvc-isystem.rst b/Help/release/dev/msvc-isystem.rst new file mode 100644 index 0000000000..4a5d79f7df --- /dev/null +++ b/Help/release/dev/msvc-isystem.rst @@ -0,0 +1,7 @@ +msvc-isystem +------------ + +* The MSVC compilers learned to pass the ``-external:I`` flag for system + includes when using the :generator:`Ninja` and :generator:`NMake Makefiles` + generators. This became available as of Visual Studio 16.10 (toolchain + version 14.29.30037). diff --git a/Modules/Compiler/MSVC-C.cmake b/Modules/Compiler/MSVC-C.cmake index 9a5104bd90..73cca36204 100644 --- a/Modules/Compiler/MSVC-C.cmake +++ b/Modules/Compiler/MSVC-C.cmake @@ -63,3 +63,9 @@ endmacro() if (CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 19.05) set(CMAKE_C_COMPILE_OPTIONS_JMC "-JMC") endif() + +# The `/external:I` flag was made non-experimental in 19.29.30036.3. +if (CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 19.29.30036.3) + set(CMAKE_INCLUDE_SYSTEM_FLAG_C "-external:I ") + set(_CMAKE_INCLUDE_SYSTEM_FLAG_C_WARNING "-external:W0 ") +endif () diff --git a/Modules/Compiler/MSVC-CXX.cmake b/Modules/Compiler/MSVC-CXX.cmake index f1c7450fad..09fe851d63 100644 --- a/Modules/Compiler/MSVC-CXX.cmake +++ b/Modules/Compiler/MSVC-CXX.cmake @@ -79,3 +79,9 @@ endif() if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 19.05) set(CMAKE_CXX_COMPILE_OPTIONS_JMC "-JMC") endif() + +# The `/external:I` flag was made non-experimental in 19.29.30036.3. +if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 19.29.30036.3) + set(CMAKE_INCLUDE_SYSTEM_FLAG_CXX "-external:I ") + set(_CMAKE_INCLUDE_SYSTEM_FLAG_CXX_WARNING "-external:W0 ") +endif () diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx index 3b282de98c..a14f0850f4 100644 --- a/Source/cmLocalGenerator.cxx +++ b/Source/cmLocalGenerator.cxx @@ -878,9 +878,12 @@ std::string cmLocalGenerator::GetIncludeFlags( // Support special system include flag if it is available and the // normal flag is repeated for each directory. cmProp sysIncludeFlag = nullptr; + cmProp sysIncludeFlagWarning = nullptr; if (repeatFlag) { sysIncludeFlag = this->Makefile->GetDefinition( cmStrCat("CMAKE_INCLUDE_SYSTEM_FLAG_", lang)); + sysIncludeFlagWarning = this->Makefile->GetDefinition( + cmStrCat("_CMAKE_INCLUDE_SYSTEM_FLAG_", lang, "_WARNING")); } cmProp fwSearchFlag = this->Makefile->GetDefinition( @@ -889,6 +892,7 @@ std::string cmLocalGenerator::GetIncludeFlags( cmStrCat("CMAKE_", lang, "_SYSTEM_FRAMEWORK_SEARCH_FLAG")); bool flagUsed = false; + bool sysIncludeFlagUsed = false; std::set emitted; #ifdef __APPLE__ emitted.insert("/System/Library/Frameworks"); @@ -915,6 +919,7 @@ std::string cmLocalGenerator::GetIncludeFlags( if (sysIncludeFlag && target && target->IsSystemIncludeDirectory(i, config, lang)) { includeFlags << *sysIncludeFlag; + sysIncludeFlagUsed = true; } else { includeFlags << includeFlag; } @@ -931,6 +936,9 @@ std::string cmLocalGenerator::GetIncludeFlags( } includeFlags << sep; } + if (sysIncludeFlagUsed && sysIncludeFlagWarning) { + includeFlags << *sysIncludeFlagWarning; + } std::string flags = includeFlags.str(); // remove trailing separators if ((sep[0] != ' ') && !flags.empty() && flags.back() == sep[0]) {