cmTarget: Fix system include annotation propagation.

Direct users of IMPORTED targets treat INTERFACE_INCLUDE_DIRECTORIES
as SYSTEM, after commit a63fcbcb (Always consider includes from IMPORTED
targets to be SYSTEM., 2013-08-29).  It was intended that transitive
use of an IMPORTED target would have the same behavior, but that
did not work.  The implementation processed only direct dependencies
in cmTarget::FinalizeSystemIncludeDirectories.

Implement transitive evaluation of dependencies by traversing the
link interface of each target in the link implementation.

Source/cmGeneratorExpressionDAGChecker.cxx

@@ -192,7 +192,8 @@ bool
 cmGeneratorExpressionDAGChecker::EvaluatingSystemIncludeDirectories() const
 {
   const char *prop = this->Property.c_str();
-  return strcmp(prop, "INTERFACE_SYSTEM_INCLUDE_DIRECTORIES") == 0;
+  return (strcmp(prop, "SYSTEM_INCLUDE_DIRECTORIES") == 0
+       || strcmp(prop, "INTERFACE_SYSTEM_INCLUDE_DIRECTORIES") == 0);
 }
 
 //----------------------------------------------------------------------------

Source/cmGeneratorTarget.cxx

@@ -18,9 +18,12 @@
 #include "cmSourceFile.h"
 #include "cmGeneratorExpression.h"
 #include "cmGeneratorExpressionDAGChecker.h"
+#include "cmComputeLinkInformation.h"
 
 #include <queue>
 
+#include "assert.h"
+
 //----------------------------------------------------------------------------
 cmGeneratorTarget::cmGeneratorTarget(cmTarget* t): Target(t)
 {
@@ -59,10 +62,50 @@ cmGeneratorTarget::GetSourceDepends(cmSourceFile* sf) const
   return 0;
 }
 
+static void handleSystemIncludesDep(cmMakefile *mf, const std::string &name,
+                                  const char *config, cmTarget *headTarget,
+                                  cmGeneratorExpressionDAGChecker *dagChecker,
+                                  std::vector<std::string>& result)
+{
+  cmTarget* depTgt = mf->FindTargetToUse(name.c_str());
+
+  if (!depTgt)
+    {
+    return;
+    }
+
+  cmListFileBacktrace lfbt;
+
+  if (const char* dirs =
+          depTgt->GetProperty("INTERFACE_SYSTEM_INCLUDE_DIRECTORIES"))
+    {
+    cmGeneratorExpression ge(lfbt);
+    cmSystemTools::ExpandListArgument(ge.Parse(dirs)
+                                      ->Evaluate(mf,
+                                      config, false, headTarget,
+                                      depTgt, dagChecker), result);
+    }
+  if (!depTgt->IsImported())
+    {
+    return;
+    }
+
+  if (const char* dirs =
+                depTgt->GetProperty("INTERFACE_INCLUDE_DIRECTORIES"))
+    {
+    cmGeneratorExpression ge(lfbt);
+    cmSystemTools::ExpandListArgument(ge.Parse(dirs)
+                                      ->Evaluate(mf,
+                                      config, false, headTarget,
+                                      depTgt, dagChecker), result);
+    }
+}
+
 //----------------------------------------------------------------------------
 bool cmGeneratorTarget::IsSystemIncludeDirectory(const char *dir,
                                                  const char *config) const
 {
+  assert(this->GetType() != cmTarget::INTERFACE_LIBRARY);
   std::string config_upper;
   if(config && *config)
     {
@@ -75,39 +118,81 @@ bool cmGeneratorTarget::IsSystemIncludeDirectory(const char *dir,
 
   if (iter == this->SystemIncludesCache.end())
     {
+    cmTarget::LinkImplementation const* impl
+                  = this->Target->GetLinkImplementation(config, this->Target);
+    if(!impl)
+      {
+      return false;
+      }
+
+    cmListFileBacktrace lfbt;
+    cmGeneratorExpressionDAGChecker dagChecker(lfbt,
+                                        this->GetName(),
+                                        "SYSTEM_INCLUDE_DIRECTORIES", 0, 0);
+
     std::vector<std::string> result;
     for (std::set<cmStdString>::const_iterator
         it = this->Target->GetSystemIncludeDirectories().begin();
         it != this->Target->GetSystemIncludeDirectories().end(); ++it)
       {
-      cmListFileBacktrace lfbt;
       cmGeneratorExpression ge(lfbt);
-
-      cmGeneratorExpressionDAGChecker dagChecker(lfbt,
-                                                this->GetName(),
-                                "INTERFACE_SYSTEM_INCLUDE_DIRECTORIES", 0, 0);
-
       cmSystemTools::ExpandListArgument(ge.Parse(*it)
                                           ->Evaluate(this->Makefile,
                                           config, false, this->Target,
                                           &dagChecker), result);
       }
+
+    std::set<cmStdString> uniqueDeps;
+    for(std::vector<std::string>::const_iterator li = impl->Libraries.begin();
+        li != impl->Libraries.end(); ++li)
+      {
+      if (uniqueDeps.insert(*li).second)
+        {
+        cmTarget* tgt = this->Makefile->FindTargetToUse(li->c_str());
+
+        if (!tgt)
+          {
+          continue;
+          }
+
+        handleSystemIncludesDep(this->Makefile, *li, config, this->Target,
+                                &dagChecker, result);
+
+        std::vector<std::string> deps;
+        tgt->GetTransitivePropertyLinkLibraries(config, this->Target, deps);
+
+        for(std::vector<std::string>::const_iterator di = deps.begin();
+            di != deps.end(); ++di)
+          {
+          if (uniqueDeps.insert(*di).second)
+            {
+            handleSystemIncludesDep(this->Makefile, *di, config, this->Target,
+                                    &dagChecker, result);
+            }
+          }
+        }
+      }
+    std::set<cmStdString> unique;
     for(std::vector<std::string>::iterator li = result.begin();
         li != result.end(); ++li)
       {
       cmSystemTools::ConvertToUnixSlashes(*li);
+      unique.insert(*li);
+      }
+    result.clear();
+    for(std::set<cmStdString>::iterator li = unique.begin();
+        li != unique.end(); ++li)
+      {
+      result.push_back(*li);
       }
 
     IncludeCacheType::value_type entry(config_upper, result);
     iter = this->SystemIncludesCache.insert(entry).first;
     }
 
-  if (std::find(iter->second.begin(),
+  std::string dirString = dir;
-                iter->second.end(), dir) != iter->second.end())
+  return std::binary_search(iter->second.begin(), iter->second.end(),
-    {
+                            dirString);
-    return true;
-    }
-  return false;
 }
 
 //----------------------------------------------------------------------------

Source/cmGlobalGenerator.cxx

@@ -1164,17 +1164,6 @@ void cmGlobalGenerator::Generate()
   // it builds by default.
   this->FillLocalGeneratorToTargetMap();
 
-  for (i = 0; i < this->LocalGenerators.size(); ++i)
-    {
-    cmMakefile* mf = this->LocalGenerators[i]->GetMakefile();
-    cmTargets* targets = &(mf->GetTargets());
-    for ( cmTargets::iterator it = targets->begin();
-        it != targets->end(); ++ it )
-      {
-      it->second.FinalizeSystemIncludeDirectories();
-      }
-    }
-
   // Generate project files
   for (i = 0; i < this->LocalGenerators.size(); ++i)
     {

Source/cmTarget.cxx

@@ -1041,62 +1041,6 @@ cmTarget::AddSystemIncludeDirectories(const std::vector<std::string> &incs)
     }
 }
 
-//----------------------------------------------------------------------------
-void cmTarget::FinalizeSystemIncludeDirectories()
-{
-  for (std::vector<cmValueWithOrigin>::const_iterator
-      it = this->Internal->LinkImplementationPropertyEntries.begin(),
-      end = this->Internal->LinkImplementationPropertyEntries.end();
-      it != end; ++it)
-    {
-    if (!cmGeneratorExpression::IsValidTargetName(it->Value)
-        && cmGeneratorExpression::Find(it->Value) == std::string::npos)
-      {
-      continue;
-      }
-    {
-    cmListFileBacktrace lfbt;
-    cmGeneratorExpression ge(lfbt);
-    cmsys::auto_ptr<cmCompiledGeneratorExpression> cge =
-                                                      ge.Parse(it->Value);
-    std::string targetName = cge->Evaluate(this->Makefile, 0,
-                                      false, this, 0, 0);
-    cmTarget *tgt = this->Makefile->FindTargetToUse(targetName.c_str());
-    if (!tgt || tgt == this)
-      {
-      continue;
-      }
-    if (tgt->IsImported()
-        && tgt->GetProperty("INTERFACE_INCLUDE_DIRECTORIES")
-        && !this->GetPropertyAsBool("NO_SYSTEM_FROM_IMPORTED"))
-      {
-      std::string includeGenex = "$<TARGET_PROPERTY:" +
-                                it->Value + ",INTERFACE_INCLUDE_DIRECTORIES>";
-      if (cmGeneratorExpression::Find(it->Value) != std::string::npos)
-        {
-        // Because it->Value is a generator expression, ensure that it
-        // evaluates to the non-empty string before being used in the
-        // TARGET_PROPERTY expression.
-        includeGenex = "$<$<BOOL:" + it->Value + ">:" + includeGenex + ">";
-        }
-      this->SystemIncludeDirectories.insert(includeGenex);
-      return; // The INTERFACE_SYSTEM_INCLUDE_DIRECTORIES are a subset
-              // of the INTERFACE_INCLUDE_DIRECTORIES
-      }
-    }
-    std::string includeGenex = "$<TARGET_PROPERTY:" +
-                        it->Value + ",INTERFACE_SYSTEM_INCLUDE_DIRECTORIES>";
-    if (cmGeneratorExpression::Find(it->Value) != std::string::npos)
-      {
-      // Because it->Value is a generator expression, ensure that it
-      // evaluates to the non-empty string before being used in the
-      // TARGET_PROPERTY expression.
-      includeGenex = "$<$<BOOL:" + it->Value + ">:" + includeGenex + ">";
-      }
-    this->SystemIncludeDirectories.insert(includeGenex);
-    }
-}
-
 //----------------------------------------------------------------------------
 void
 cmTarget::AnalyzeLibDependencies( const cmMakefile& mf )

Source/cmTarget.h

@@ -568,8 +568,6 @@ public:
   std::set<cmStdString> const & GetSystemIncludeDirectories() const
     { return this->SystemIncludeDirectories; }
 
-  void FinalizeSystemIncludeDirectories();
-
   bool LinkLanguagePropagatesToDependents() const
   { return this->TargetTypeValue == STATIC_LIBRARY; }
 

Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt

@@ -25,6 +25,10 @@ add_library(imported_consumer imported_consumer.cpp)
 target_link_libraries(imported_consumer iface)
 target_compile_options(imported_consumer PRIVATE -Werror=unused-variable)
 
+add_library(imported_consumer2 imported_consumer.cpp)
+target_link_libraries(imported_consumer2 imported_consumer)
+target_compile_options(imported_consumer2 PRIVATE -Werror=unused-variable)
+
 macro(do_try_compile error_option)
   set(TC_ARGS
     IFACE_TRY_COMPILE_${error_option}