From ec2c67bcf3aada9b601d5dca52b31a4fb6104240 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Mon, 18 Feb 2013 11:24:41 +0100 Subject: Strip stray semicolons when evaluating generator expressions. --- Source/cmGeneratorExpression.cxx | 7 ++++--- Source/cmGeneratorExpression.h | 1 + Source/cmGeneratorExpressionEvaluator.cxx | 3 +++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx index 5d162fe..08ffe1d 100644 --- a/Source/cmGeneratorExpression.cxx +++ b/Source/cmGeneratorExpression.cxx @@ -153,7 +153,8 @@ cmCompiledGeneratorExpression::~cmCompiledGeneratorExpression() } //---------------------------------------------------------------------------- -static std::string stripEmptyListElements(const std::string &input) +std::string cmGeneratorExpression::StripEmptyListElements( + const std::string &input) { std::string result; @@ -223,7 +224,7 @@ static std::string stripAllGeneratorExpressions(const std::string &input) lastPos = pos; } result += input.substr(lastPos); - return stripEmptyListElements(result); + return cmGeneratorExpression::StripEmptyListElements(result); } //---------------------------------------------------------------------------- @@ -284,7 +285,7 @@ static std::string stripExportInterface(const std::string &input, } result += input.substr(lastPos); - return stripEmptyListElements(result); + return cmGeneratorExpression::StripEmptyListElements(result); } //---------------------------------------------------------------------------- diff --git a/Source/cmGeneratorExpression.h b/Source/cmGeneratorExpression.h index 489b052..ca41573 100644 --- a/Source/cmGeneratorExpression.h +++ b/Source/cmGeneratorExpression.h @@ -66,6 +66,7 @@ public: static bool IsValidTargetName(const std::string &input); + static std::string StripEmptyListElements(const std::string &input); private: cmGeneratorExpression(const cmGeneratorExpression &); void operator=(const cmGeneratorExpression &); diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index cd6a40b..023daf8 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -508,6 +508,9 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode } } + linkedTargetsContent = + cmGeneratorExpression::StripEmptyListElements(linkedTargetsContent); + if (!prop) { if (target->IsImported()) -- cgit v0.12 From e72eaadc42be80ef8273addfc19a6dd13b5feb90 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 22 Feb 2013 17:16:19 +0100 Subject: Workaround broken code where a target has itself in its link iface. There is a test for this since commit 8e756d2b (Tolerate cycles in shared library link interfaces (#12647), 2012-01-12), so make sure it continues to pass, even as we require no self-references in new INTERFACE_ property generator expressions. --- Source/cmGeneratorExpressionEvaluator.cxx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index 023daf8..3407187 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -471,6 +471,13 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode if (propertyName == "INTERFACE_INCLUDE_DIRECTORIES" || propertyName == "INTERFACE_COMPILE_DEFINITIONS") { + if (*it == target->GetName()) + { + // Broken code can have a target in its own link interface. + // Don't follow such link interface entries so as not to create a + // self-referencing loop. + continue; + } const cmTarget::LinkInterface *iface = target->GetLinkInterface( context->Config, context->HeadTarget); -- cgit v0.12 From d1a2729b1af86a0a3abfb21df18ed85bcfaa59c6 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Mon, 18 Feb 2013 11:08:24 +0100 Subject: Fix DAG checker finding cycling dependencies. Before this patch, the following is reported falsely as a self-reference: target_link_libraries(empty2 LINK_PUBLIC empty3) target_link_libraries(empty3 LINK_PUBLIC empty2) add_custom_target(... -DINCLUDES=$ ) The reason is that the existing code assumed that all reading of include directories would be done through cmTarget::GetIncludeDirectories() and would therefore be initialized with a DagChecker. That is not the case if reading the property with an 'external' generator expression. --- Source/cmGeneratorExpressionDAGChecker.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index 57e7358..2d50c25 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -126,7 +126,7 @@ cmGeneratorExpressionDAGChecker::checkGraph() const { if (this->Target == parent->Target && this->Property == parent->Property) { - return parent->Parent ? CYCLIC_REFERENCE : SELF_REFERENCE; + return (parent == this->Parent) ? SELF_REFERENCE : CYCLIC_REFERENCE; } parent = parent->Parent; } -- cgit v0.12 From 7e707444be6f7344888102ce4f88db48a31cab63 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 13 Feb 2013 12:35:31 +0100 Subject: Expand includes and defines transitively in 'external' genexes. This means that we can use expressions of the form $ to get a list of the interface include directories of foo, including those coming from dependencies. We can't have a test of a target which has a single include directory in its INCLUDE_DIRECTORIES because the shell on the MSYS platforms transforms a single include directory to include a prefix, which is not what the test expects. We test a target with two directories instead as a means to test a target with no link dependencies. --- Source/cmGeneratorExpressionEvaluator.cxx | 87 +++++++++++++++++------------ Tests/GeneratorExpression/CMakeLists.txt | 31 +++++++++- Tests/GeneratorExpression/check-part2.cmake | 8 +++ Tests/GeneratorExpression/empty.cpp | 2 + 4 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 Tests/GeneratorExpression/empty.cpp diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index 3407187..683245c 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -452,8 +452,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode const char *prop = target->GetProperty(propertyName.c_str()); - std::string linkedTargetsContent; - if (dagCheckerParent) { if (dagCheckerParent->EvaluatingLinkLibraries()) @@ -467,9 +465,40 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode { assert(dagCheckerParent->EvaluatingIncludeDirectories() || dagCheckerParent->EvaluatingCompileDefinitions()); + } + } + + std::string linkedTargetsContent; + + std::string interfacePropertyName; - if (propertyName == "INTERFACE_INCLUDE_DIRECTORIES" - || propertyName == "INTERFACE_COMPILE_DEFINITIONS") + if (propertyName == "INTERFACE_INCLUDE_DIRECTORIES" + || propertyName == "INCLUDE_DIRECTORIES") + { + interfacePropertyName = "INTERFACE_INCLUDE_DIRECTORIES"; + } + else if (propertyName == "INTERFACE_COMPILE_DEFINITIONS" + || propertyName == "COMPILE_DEFINITIONS" + || strncmp(propertyName.c_str(), "COMPILE_DEFINITIONS_", 20) == 0) + { + interfacePropertyName = "INTERFACE_COMPILE_DEFINITIONS"; + } + + if (interfacePropertyName == "INTERFACE_INCLUDE_DIRECTORIES" + || interfacePropertyName == "INTERFACE_COMPILE_DEFINITIONS") + { + const cmTarget::LinkInterface *iface = target->GetLinkInterface( + context->Config, + context->HeadTarget); + if(iface) + { + cmGeneratorExpression ge(context->Backtrace); + + std::string sep; + std::string depString; + for (std::vector::const_iterator + it = iface->Libraries.begin(); + it != iface->Libraries.end(); ++it) { if (*it == target->GetName()) { @@ -478,40 +507,26 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode // self-referencing loop. continue; } - const cmTarget::LinkInterface *iface = target->GetLinkInterface( - context->Config, - context->HeadTarget); - if(iface) + if (context->Makefile->FindTargetToUse(it->c_str())) { - cmGeneratorExpression ge(context->Backtrace); - - std::string sep; - std::string depString; - for (std::vector::const_iterator - it = iface->Libraries.begin(); - it != iface->Libraries.end(); ++it) - { - if (context->Makefile->FindTargetToUse(it->c_str())) - { - depString += - sep + "$"; - sep = ";"; - } - } - cmsys::auto_ptr cge = - ge.Parse(depString); - linkedTargetsContent = cge->Evaluate(context->Makefile, - context->Config, - context->Quiet, - context->HeadTarget, - target, - &dagChecker); - if (cge->GetHadContextSensitiveCondition()) - { - context->HadContextSensitiveCondition = true; - } + depString += + sep + "$"; + sep = ";"; } } + cmsys::auto_ptr cge = + ge.Parse(depString); + linkedTargetsContent = cge->Evaluate(context->Makefile, + context->Config, + context->Quiet, + context->HeadTarget, + target, + &dagChecker); + if (cge->GetHadContextSensitiveCondition()) + { + context->HadContextSensitiveCondition = true; + } } } @@ -551,7 +566,7 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode sizeof(*targetPropertyTransitiveWhitelist)); ++i) { - if (targetPropertyTransitiveWhitelist[i] == propertyName) + if (targetPropertyTransitiveWhitelist[i] == interfacePropertyName) { cmGeneratorExpression ge(context->Backtrace); cmsys::auto_ptr cge = ge.Parse(prop); diff --git a/Tests/GeneratorExpression/CMakeLists.txt b/Tests/GeneratorExpression/CMakeLists.txt index ecbbedf..fff7c87 100644 --- a/Tests/GeneratorExpression/CMakeLists.txt +++ b/Tests/GeneratorExpression/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required (VERSION 2.8.8) -project(GeneratorExpression NONE) +project(GeneratorExpression CXX) add_custom_target(check-part1 ALL COMMAND ${CMAKE_COMMAND} @@ -62,6 +62,27 @@ add_custom_target(check-part1 ALL VERBATIM ) +add_library(empty1 empty.cpp) +target_include_directories(empty1 PUBLIC /empty1/public) +target_include_directories(empty1 PRIVATE /empty1/private) + +add_library(empty2 empty.cpp) +target_include_directories(empty2 PUBLIC /empty2/public) + +add_library(empty3 empty.cpp) +target_include_directories(empty3 PUBLIC /empty3/public) +target_include_directories(empty3 PRIVATE /empty3/private) + +add_library(empty4 empty.cpp) +target_include_directories(empty4 PUBLIC /empty4/public) + +target_link_libraries(empty1 LINK_PUBLIC empty2) +target_link_libraries(empty2 LINK_PUBLIC empty3 empty4) +target_link_libraries(empty3 LINK_PUBLIC empty2 empty4) + +add_library(empty5 empty.cpp) +target_include_directories(empty5 PRIVATE /empty5/private1 /empty5/private2) + add_custom_target(check-part2 ALL COMMAND ${CMAKE_COMMAND} -Dtest_incomplete_1=$< @@ -89,6 +110,14 @@ add_custom_target(check-part2 ALL -Dtest_install_interface=$ -Dtest_target_name_1=$ -Dtest_target_name_2=$ + -Dtest_target_includes1=$ + -Dtest_target_includes2=$ + -Dtest_target_includes3=$ + -Dtest_target_includes4=$ + -Dtest_target_includes5=$ + -Dtest_target_includes6=$ + -Dtest_target_includes7=$ + -Dtest_target_includes8=$ -P ${CMAKE_CURRENT_SOURCE_DIR}/check-part2.cmake COMMAND ${CMAKE_COMMAND} -E echo "check done (part 2 of 2)" VERBATIM diff --git a/Tests/GeneratorExpression/check-part2.cmake b/Tests/GeneratorExpression/check-part2.cmake index 8855a97..44ded62 100644 --- a/Tests/GeneratorExpression/check-part2.cmake +++ b/Tests/GeneratorExpression/check-part2.cmake @@ -26,3 +26,11 @@ check(test_build_interface "build") check(test_install_interface "") check(test_target_name_1 "tgt,ok") check(test_target_name_2 "tgt:ok") +check(test_target_includes1 "/empty1/public;/empty2/public;/empty3/public;/empty4/public;/empty4/public") +check(test_target_includes2 "/empty2/public;/empty3/public;/empty4/public;/empty4/public") +check(test_target_includes3 "/empty3/public;/empty2/public;/empty4/public;/empty4/public") +check(test_target_includes4 "/empty1/public;/empty1/private;/empty2/public;/empty3/public;/empty4/public") +check(test_target_includes5 "/empty2/public;/empty3/public;/empty2/public;/empty4/public") +check(test_target_includes6 "/empty3/public;/empty3/private;/empty2/public;/empty3/public;/empty4/public") +check(test_target_includes7 "/empty1/public;/empty2/public;/empty3/public;/empty4/public;/empty4/public") +check(test_target_includes8 "/empty5/private1;/empty5/private2") diff --git a/Tests/GeneratorExpression/empty.cpp b/Tests/GeneratorExpression/empty.cpp new file mode 100644 index 0000000..c539901 --- /dev/null +++ b/Tests/GeneratorExpression/empty.cpp @@ -0,0 +1,2 @@ + +// empty -- cgit v0.12 From 98a672528d7e6c192fc75abb10161121c8b28073 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Mon, 18 Feb 2013 10:34:16 +0100 Subject: Fix constness of accessors. --- Source/cmGeneratorExpressionDAGChecker.cxx | 4 ++-- Source/cmGeneratorExpressionDAGChecker.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index 2d50c25..f8776d9 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -153,7 +153,7 @@ bool cmGeneratorExpressionDAGChecker::EvaluatingLinkLibraries() } //---------------------------------------------------------------------------- -bool cmGeneratorExpressionDAGChecker::EvaluatingIncludeDirectories() +bool cmGeneratorExpressionDAGChecker::EvaluatingIncludeDirectories() const { const char *prop = this->Property.c_str(); return (strcmp(prop, "INCLUDE_DIRECTORIES") == 0 @@ -161,7 +161,7 @@ bool cmGeneratorExpressionDAGChecker::EvaluatingIncludeDirectories() } //---------------------------------------------------------------------------- -bool cmGeneratorExpressionDAGChecker::EvaluatingCompileDefinitions() +bool cmGeneratorExpressionDAGChecker::EvaluatingCompileDefinitions() const { const char *prop = this->Property.c_str(); return (strcmp(prop, "COMPILE_DEFINITIONS") == 0 diff --git a/Source/cmGeneratorExpressionDAGChecker.h b/Source/cmGeneratorExpressionDAGChecker.h index a2e5ce4..62a5cdf 100644 --- a/Source/cmGeneratorExpressionDAGChecker.h +++ b/Source/cmGeneratorExpressionDAGChecker.h @@ -38,8 +38,8 @@ struct cmGeneratorExpressionDAGChecker const std::string &expr); bool EvaluatingLinkLibraries(); - bool EvaluatingIncludeDirectories(); - bool EvaluatingCompileDefinitions(); + bool EvaluatingIncludeDirectories() const; + bool EvaluatingCompileDefinitions() const; private: Result checkGraph() const; -- cgit v0.12 From 8dfdf1c734af19a1e49efa4568e5e1f8fc7cb2f2 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Mon, 18 Feb 2013 10:42:00 +0100 Subject: Fix the tests for evaluating includes and defines. We should also check whether the INTERFACE_ variant of a property is being read, and in the case of the compile definitions, we should test the _ suffixed variants. That is already available through the use of the methods. This way, we use the ALREADY_SEEN optimization when evaluating the includes of a target in 'external' generator expressions, ie, those used in a add_custom_command invokation, as opposed to evaluating the INCLUDE_DIRECTORIES of a target itself via GetIncludeDirectories. --- Source/cmGeneratorExpressionDAGChecker.cxx | 4 ++-- Tests/GeneratorExpression/check-part2.cmake | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx index f8776d9..5cb50b9 100644 --- a/Source/cmGeneratorExpressionDAGChecker.cxx +++ b/Source/cmGeneratorExpressionDAGChecker.cxx @@ -33,8 +33,8 @@ cmGeneratorExpressionDAGChecker::cmGeneratorExpressionDAGChecker( } this->CheckResult = this->checkGraph(); - if (CheckResult == DAG && (top->Property == "INCLUDE_DIRECTORIES" - || top->Property == "COMPILE_DEFINITIONS") ) + if (CheckResult == DAG && (top->EvaluatingIncludeDirectories() + || top->EvaluatingCompileDefinitions())) { std::map >::const_iterator it = top->Seen.find(target); diff --git a/Tests/GeneratorExpression/check-part2.cmake b/Tests/GeneratorExpression/check-part2.cmake index 44ded62..3f7187c 100644 --- a/Tests/GeneratorExpression/check-part2.cmake +++ b/Tests/GeneratorExpression/check-part2.cmake @@ -26,11 +26,11 @@ check(test_build_interface "build") check(test_install_interface "") check(test_target_name_1 "tgt,ok") check(test_target_name_2 "tgt:ok") -check(test_target_includes1 "/empty1/public;/empty2/public;/empty3/public;/empty4/public;/empty4/public") -check(test_target_includes2 "/empty2/public;/empty3/public;/empty4/public;/empty4/public") -check(test_target_includes3 "/empty3/public;/empty2/public;/empty4/public;/empty4/public") +check(test_target_includes1 "/empty1/public;/empty2/public;/empty3/public;/empty4/public") +check(test_target_includes2 "/empty2/public;/empty3/public;/empty4/public") +check(test_target_includes3 "/empty3/public;/empty2/public;/empty4/public") check(test_target_includes4 "/empty1/public;/empty1/private;/empty2/public;/empty3/public;/empty4/public") check(test_target_includes5 "/empty2/public;/empty3/public;/empty2/public;/empty4/public") check(test_target_includes6 "/empty3/public;/empty3/private;/empty2/public;/empty3/public;/empty4/public") -check(test_target_includes7 "/empty1/public;/empty2/public;/empty3/public;/empty4/public;/empty4/public") +check(test_target_includes7 "/empty1/public;/empty2/public;/empty3/public;/empty4/public") check(test_target_includes8 "/empty5/private1;/empty5/private2") -- cgit v0.12