From 76ad2ecb500c8652e59179959b8ecee28a5196de Mon Sep 17 00:00:00 2001
From: Ephi Sinowitz <esinowitz@bloomberg.net>
Date: Fri, 13 Apr 2018 07:58:47 -0400
Subject: Order SYSTEM include directories after non-system directories

An effect of the `-isystem` flag is to search the directory after those
specified via `-I` flags.  Make behavior more consistent on compilers
that do not have any `-isystem` flag by explicitly moving system include
directories to the end.
---
 Help/release/dev/reorder-sys-includes.rst          |  7 +++++++
 Source/cmLocalGenerator.cxx                        | 23 +++++++++++++++++++++-
 Source/cmLocalGenerator.h                          |  4 ++++
 Tests/IncludeDirectories/CMakeLists.txt            |  4 ++++
 .../SystemIncludeDirectories/systemlib/ordertest.h |  1 +
 .../SystemIncludeDirectories/userlib/ordertest.h   |  1 +
 Tests/IncludeDirectories/ordertest.cpp             |  1 +
 7 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 Help/release/dev/reorder-sys-includes.rst
 create mode 100644 Tests/IncludeDirectories/SystemIncludeDirectories/systemlib/ordertest.h
 create mode 100644 Tests/IncludeDirectories/SystemIncludeDirectories/userlib/ordertest.h
 create mode 100644 Tests/IncludeDirectories/ordertest.cpp

diff --git a/Help/release/dev/reorder-sys-includes.rst b/Help/release/dev/reorder-sys-includes.rst
new file mode 100644
index 0000000..14d520f
--- /dev/null
+++ b/Help/release/dev/reorder-sys-includes.rst
@@ -0,0 +1,7 @@
+reorder-sys-includes
+--------------------
+
+* Include directories marked as ``SYSTEM`` are now moved after non-system
+  directories.  The ``-isystem`` flag does this automatically, so moving
+  them explicitly to the end makes the behavior consistent on compilers
+  that do not have any ``-isystem`` flag.
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 629d54a..c5370e4 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -202,6 +202,22 @@ void cmLocalGenerator::ComputeObjectMaxPath()
   this->ObjectMaxPathViolations.clear();
 }
 
+void cmLocalGenerator::MoveSystemIncludesToEnd(
+  std::vector<std::string>& includeDirs, const std::string& config,
+  const std::string& lang, const cmGeneratorTarget* target) const
+{
+  if (!target) {
+    return;
+  }
+
+  std::stable_sort(
+    includeDirs.begin(), includeDirs.end(),
+    [&target, &config, &lang](std::string const& a, std::string const& b) {
+      return !target->IsSystemIncludeDirectory(a, config, lang) &&
+        target->IsSystemIncludeDirectory(b, config, lang);
+    });
+}
+
 void cmLocalGenerator::TraceDependencies()
 {
   std::vector<std::string> configs;
@@ -651,7 +667,7 @@ std::string cmLocalGenerator::ConvertToIncludeReference(
 }
 
 std::string cmLocalGenerator::GetIncludeFlags(
-  const std::vector<std::string>& includes, cmGeneratorTarget* target,
+  const std::vector<std::string>& includeDirs, cmGeneratorTarget* target,
   const std::string& lang, bool forceFullPaths, bool forResponseFile,
   const std::string& config)
 {
@@ -659,6 +675,9 @@ std::string cmLocalGenerator::GetIncludeFlags(
     return "";
   }
 
+  std::vector<std::string> includes = includeDirs;
+  this->MoveSystemIncludesToEnd(includes, config, lang, target);
+
   OutputFormat shellFormat = forResponseFile ? RESPONSE : SHELL;
   std::ostringstream includeFlags;
 
@@ -924,6 +943,8 @@ void cmLocalGenerator::GetIncludeDirectories(std::vector<std::string>& dirs,
     }
   }
 
+  this->MoveSystemIncludesToEnd(dirs, config, lang, target);
+
   // Add standard include directories for this language.
   // We do not filter out implicit directories here.
   std::string const standardIncludesVar =
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index 533ac56..9ba62cc 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -411,6 +411,10 @@ private:
                                    int targetType);
 
   void ComputeObjectMaxPath();
+  void MoveSystemIncludesToEnd(std::vector<std::string>& includeDirs,
+                               const std::string& config,
+                               const std::string& lang,
+                               cmGeneratorTarget const* target) const;
 };
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)
diff --git a/Tests/IncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/CMakeLists.txt
index db18462..b7b8320 100644
--- a/Tests/IncludeDirectories/CMakeLists.txt
+++ b/Tests/IncludeDirectories/CMakeLists.txt
@@ -65,6 +65,10 @@ else()
     PROPERTIES COMPILE_FLAGS "-ITarProp")
 endif()
 
+add_library(ordertest ordertest.cpp)
+target_include_directories(ordertest SYSTEM PUBLIC SystemIncludeDirectories/systemlib)
+target_include_directories(ordertest PUBLIC SystemIncludeDirectories/userlib)
+
 add_subdirectory(StandardIncludeDirectories)
 add_subdirectory(TargetIncludeDirectories)
 
diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/systemlib/ordertest.h b/Tests/IncludeDirectories/SystemIncludeDirectories/systemlib/ordertest.h
new file mode 100644
index 0000000..28915e6
--- /dev/null
+++ b/Tests/IncludeDirectories/SystemIncludeDirectories/systemlib/ordertest.h
@@ -0,0 +1 @@
+#error ordertest.h includes from systemlib
diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/userlib/ordertest.h b/Tests/IncludeDirectories/SystemIncludeDirectories/userlib/ordertest.h
new file mode 100644
index 0000000..fa882cb
--- /dev/null
+++ b/Tests/IncludeDirectories/SystemIncludeDirectories/userlib/ordertest.h
@@ -0,0 +1 @@
+/* empty file */
diff --git a/Tests/IncludeDirectories/ordertest.cpp b/Tests/IncludeDirectories/ordertest.cpp
new file mode 100644
index 0000000..7e24f77
--- /dev/null
+++ b/Tests/IncludeDirectories/ordertest.cpp
@@ -0,0 +1 @@
+#include "ordertest.h"
-- 
cgit v0.12