From faedd2bea9c98fddd9e9f70deebdb53f8f369124 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 1 Jan 2014 15:49:05 +0100 Subject: [PATCH] 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 | 3 +- Source/cmGeneratorTarget.cxx | 115 +++++++++++++++--- Source/cmGlobalGenerator.cxx | 11 -- Source/cmTarget.cxx | 56 --------- Source/cmTarget.h | 2 - .../SystemIncludeDirectories/CMakeLists.txt | 4 + 6 files changed, 106 insertions(+), 85 deletions(-) diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index 84d7735b2..c2c4e20f4 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/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); } //---------------------------------------------------------------------------- diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index fdd4e6dcd..6894cfcca 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -18,9 +18,12 @@ #include "cmSourceFile.h" #include "cmGeneratorExpression.h" #include "cmGeneratorExpressionDAGChecker.h" +#include "cmComputeLinkInformation.h" #include +#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& 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 result; for (std::set::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); + ->Evaluate(this->Makefile, + config, false, this->Target, + &dagChecker), result); } + + std::set uniqueDeps; + for(std::vector::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 deps; + tgt->GetTransitivePropertyLinkLibraries(config, this->Target, deps); + + for(std::vector::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 unique; for(std::vector::iterator li = result.begin(); li != result.end(); ++li) { cmSystemTools::ConvertToUnixSlashes(*li); + unique.insert(*li); + } + result.clear(); + for(std::set::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(), - iter->second.end(), dir) != iter->second.end()) - { - return true; - } - return false; + std::string dirString = dir; + return std::binary_search(iter->second.begin(), iter->second.end(), + dirString); } //---------------------------------------------------------------------------- diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 0a5677139..226a45a50 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/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) { diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 4b7bd3b57..b06480b47 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1041,62 +1041,6 @@ cmTarget::AddSystemIncludeDirectories(const std::vector &incs) } } -//---------------------------------------------------------------------------- -void cmTarget::FinalizeSystemIncludeDirectories() -{ - for (std::vector::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 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 = "$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 = "$<$Value + ">:" + includeGenex + ">"; - } - this->SystemIncludeDirectories.insert(includeGenex); - return; // The INTERFACE_SYSTEM_INCLUDE_DIRECTORIES are a subset - // of the INTERFACE_INCLUDE_DIRECTORIES - } - } - std::string includeGenex = "$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 = "$<$Value + ">:" + includeGenex + ">"; - } - this->SystemIncludeDirectories.insert(includeGenex); - } -} - //---------------------------------------------------------------------------- void cmTarget::AnalyzeLibDependencies( const cmMakefile& mf ) diff --git a/Source/cmTarget.h b/Source/cmTarget.h index e026c59fa..491664831 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -568,8 +568,6 @@ public: std::set const & GetSystemIncludeDirectories() const { return this->SystemIncludeDirectories; } - void FinalizeSystemIncludeDirectories(); - bool LinkLanguagePropagatesToDependents() const { return this->TargetTypeValue == STATIC_LIBRARY; } diff --git a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt b/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt index 1f5c93b44..abe9f74d9 100644 --- a/Tests/IncludeDirectories/SystemIncludeDirectories/CMakeLists.txt +++ b/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}