Merge topic 'msvc-isystem'

f29e1874ad Compiler/MSVC: use the `-external:I` flag for system includes
5a5c85dffd Tests/IncludeDirectories: support MSVC in system include tests
399a3204bb Tests/IncludeDirectories: align sibling predicates
20ab49193b Tests/IncludeDirectories: factor out applying flags to targets
809f7b0c3a Tests/IncludeDirectories: fix copy pasta for otherlib
b094324948 Tests/IncludeDirectories: Include system headers via angle brackets
8f63f3b04e cmVisualStudio10TargetGenerator: remove unused variable

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Julien Waechter <greenjava@gmail.com>
Acked-by: DE-VS wenglor <devs.wenglor@gmail.com>
Merge-request: !4766

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).

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 ()

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 ()

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<std::string> 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]) {

Source/cmVisualStudio10TargetGenerator.cxx

@@ -3543,8 +3543,6 @@ void cmVisualStudio10TargetGenerator::WriteNasmOptions(
   }
   Elem e2(e1, "NASM");
 
-  std::vector<std::string> includes =
-    this->GetIncludes(configName, "ASM_NASM");
   OptionsHelper nasmOptions(*(this->NasmOptions[configName]), e2);
   nasmOptions.OutputAdditionalIncludeDirectories("ASM_NASM");
   nasmOptions.OutputFlagMap();

Tests/IncludeDirectories/CMakeLists.txt

@@ -2,17 +2,25 @@ 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
+    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)
+  if ("x${CMAKE_C_COMPILER_ID}" STREQUAL "xMSVC")
-  check_cxx_compiler_flag(-Wunused-variable run_sys_includes_test)
+    set(run_sys_includes_test 1)
-  if(run_sys_includes_test)
+  else ()
-    # The Bullseye wrapper appears to break the -isystem effect.
+    include(CheckCXXCompilerFlag)
-    execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version OUTPUT_VARIABLE out ERROR_VARIABLE out)
+    check_cxx_compiler_flag(-Wunused-variable run_sys_includes_test)
-    if("x${out}" MATCHES "Bullseye")
+    if(run_sys_includes_test)
-      set(run_sys_includes_test 0)
+      # 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)

Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt

@@ -6,9 +6,17 @@ project(SystemIncludeDirectories)
 add_library(systemlib systemlib.cpp)
 target_include_directories(systemlib PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/systemlib")
 
+function (apply_error_flags target)
+  if (MSVC)
+    target_compile_options(${target} PRIVATE /we4101)
+  else ()
+    target_compile_options(${target} PRIVATE -Werror=unused-variable)
+  endif ()
+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
   $<TARGET_PROPERTY:systemlib,INTERFACE_INCLUDE_DIRECTORIES>
@@ -29,7 +37,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 +46,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(somelib PRIVATE -Werror=unused-variable)
+apply_error_flags(otherlib)
 
 macro(do_try_compile error_option)
   set(TC_ARGS
@@ -61,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()

Tests/IncludeDirectories/SystemIncludeDirectories/consumer.cpp

@@ -1,5 +1,6 @@
 
-#include "config_iface.h"
+#include <config_iface.h>
+
 #include "upstream.h"
 
 int consumer()

Tests/IncludeDirectories/SystemIncludeDirectories/imported_consumer.cpp

@@ -1,5 +1,5 @@
 
-#include "systemlib.h"
+#include <systemlib.h>
 
 int main()
 {

Tests/IncludeDirectories/SystemIncludeDirectories/upstream.h

@@ -2,7 +2,7 @@
 #ifndef UPSTREAM_H
 #define UPSTREAM_H
 
-#include "systemlib.h"
+#include <systemlib.h>
 
 #ifdef _WIN32
 __declspec(dllexport)

Tests/IncludeDirectories/SystemIncludeDirectoriesPerLang/CMakeLists.txt

@@ -7,14 +7,14 @@ set_target_properties(c_interface PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES "$<$<COMPILE_LANGUAGE:C>:${CMAKE_CURRENT_SOURCE_DIR}>"
   INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "$<$<COMPILE_LANGUAGE:C>:${CMAKE_CURRENT_SOURCE_DIR}>"
 )
-target_compile_options(c_interface INTERFACE "$<$<COMPILE_LANG_AND_ID:C,GNU,Clang>:-Werror=unused-variable>")
+target_compile_options(c_interface INTERFACE "$<$<COMPILE_LANG_AND_ID:C,GNU,Clang>:-Werror=unused-variable>;$<$<COMPILE_LANG_AND_ID:C,MSVC>:/we4101>")
 
 add_library(cxx_interface INTERFACE)
 set_target_properties(cxx_interface PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES "$<$<COMPILE_LANGUAGE:CXX>:${CMAKE_CURRENT_SOURCE_DIR}/cxx_system_include>"
   INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "$<$<COMPILE_LANGUAGE:CXX>:${CMAKE_CURRENT_SOURCE_DIR}/cxx_system_include>"
 )
-target_compile_options(cxx_interface INTERFACE "$<$<COMPILE_LANG_AND_ID:CXX,GNU,Clang>:-Werror=unused-variable>")
+target_compile_options(cxx_interface INTERFACE "$<$<COMPILE_LANG_AND_ID:CXX,GNU,Clang>:-Werror=unused-variable>;$<$<COMPILE_LANG_AND_ID:C,MSVC>:/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