Merge topic 'interface-property-external-read'

8dfdf1c Fix the tests for evaluating includes and defines.
98a6725 Fix constness of accessors.
7e70744 Expand includes and defines transitively in 'external' genexes.
d1a2729 Fix DAG checker finding cycling dependencies.
e72eaad Workaround broken code where a target has itself in its link iface.
ec2c67b Strip stray semicolons when evaluating generator expressions.

8 files changed, 113 insertions, 47 deletions

diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx
index 51ebddb..34d80ec 100644
--- a/Source/cmGeneratorExpression.cxx
+++ b/Source/cmGeneratorExpression.cxx
@@ -154,7 +154,8 @@ cmCompiledGeneratorExpression::~cmCompiledGeneratorExpression()
 }
 
 //----------------------------------------------------------------------------
-static std::string stripEmptyListElements(const std::string &input)
+std::string cmGeneratorExpression::StripEmptyListElements(
+                                                    const std::string &input)
 {
   std::string result;
 
@@ -224,7 +225,7 @@ static std::string stripAllGeneratorExpressions(const std::string &input)
     lastPos = pos;
     }
   result += input.substr(lastPos);
-  return stripEmptyListElements(result);
+  return cmGeneratorExpression::StripEmptyListElements(result);
 }
 
 //----------------------------------------------------------------------------
@@ -285,7 +286,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 e223034..86b6f25 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/cmGeneratorExpressionDAGChecker.cxx b/Source/cmGeneratorExpressionDAGChecker.cxx
index 57e7358..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<cmStdString, std::set<cmStdString> >::const_iterator it
                                                     = top->Seen.find(target);
@@ -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;
     }
@@ -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;
diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx
index 97ba524..63d4126 100644
--- a/Source/cmGeneratorExpressionEvaluator.cxx
+++ b/Source/cmGeneratorExpressionEvaluator.cxx
@@ -453,8 +453,6 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
 
     const char *prop = target->GetProperty(propertyName.c_str());
 
-    std::string linkedTargetsContent;
-
     if (dagCheckerParent)
       {
       if (dagCheckerParent->EvaluatingLinkLibraries())
@@ -468,47 +466,74 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
         {
         assert(dagCheckerParent->EvaluatingIncludeDirectories()
             || dagCheckerParent->EvaluatingCompileDefinitions());
+        }
+      }
+
+    std::string linkedTargetsContent;
+
+    std::string interfacePropertyName;
+
+    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);
 
-        if (propertyName == "INTERFACE_INCLUDE_DIRECTORIES"
-            || propertyName == "INTERFACE_COMPILE_DEFINITIONS")
+        std::string sep;
+        std::string depString;
+        for (std::vector<std::string>::const_iterator
+            it = iface->Libraries.begin();
+            it != iface->Libraries.end(); ++it)
           {
-          const cmTarget::LinkInterface *iface = target->GetLinkInterface(
-                                                        context->Config,
-                                                        context->HeadTarget);
-          if(iface)
+          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;
+            }
+          if (context->Makefile->FindTargetToUse(it->c_str()))
             {
-            cmGeneratorExpression ge(context->Backtrace);
-
-            std::string sep;
-            std::string depString;
-            for (std::vector<std::string>::const_iterator
-                it = iface->Libraries.begin();
-                it != iface->Libraries.end(); ++it)
-              {
-              if (context->Makefile->FindTargetToUse(it->c_str()))
-                {
-                depString +=
-                  sep + "$<TARGET_PROPERTY:" + *it + "," + propertyName + ">";
-                sep = ";";
-                }
-              }
-            cmsys::auto_ptr<cmCompiledGeneratorExpression> 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 + "$<TARGET_PROPERTY:" + *it + ","
+                                         + interfacePropertyName + ">";
+            sep = ";";
             }
           }
+        cmsys::auto_ptr<cmCompiledGeneratorExpression> cge =
+                                                      ge.Parse(depString);
+        linkedTargetsContent = cge->Evaluate(context->Makefile,
+                            context->Config,
+                            context->Quiet,
+                            context->HeadTarget,
+                            target,
+                            &dagChecker);
+        if (cge->GetHadContextSensitiveCondition())
+          {
+          context->HadContextSensitiveCondition = true;
+          }
         }
       }
 
+    linkedTargetsContent =
+          cmGeneratorExpression::StripEmptyListElements(linkedTargetsContent);
+
     if (!prop)
       {
       if (target->IsImported())
@@ -542,7 +567,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<cmCompiledGeneratorExpression> 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=$<INSTALL_INTERFACE:install>
     -Dtest_target_name_1=$<TARGET_NAME:tgt,ok>
     -Dtest_target_name_2=$<TARGET_NAME:tgt:ok>
+    -Dtest_target_includes1=$<TARGET_PROPERTY:empty1,INTERFACE_INCLUDE_DIRECTORIES>
+    -Dtest_target_includes2=$<TARGET_PROPERTY:empty2,INTERFACE_INCLUDE_DIRECTORIES>
+    -Dtest_target_includes3=$<TARGET_PROPERTY:empty3,INTERFACE_INCLUDE_DIRECTORIES>
+    -Dtest_target_includes4=$<TARGET_PROPERTY:empty1,INCLUDE_DIRECTORIES>
+    -Dtest_target_includes5=$<TARGET_PROPERTY:empty2,INCLUDE_DIRECTORIES>
+    -Dtest_target_includes6=$<TARGET_PROPERTY:empty3,INCLUDE_DIRECTORIES>
+    -Dtest_target_includes7=$<TARGET_PROPERTY:empty1,INTERFACE_INCLUDE_DIRECTORIES>
+    -Dtest_target_includes8=$<TARGET_PROPERTY:empty5,INCLUDE_DIRECTORIES>
     -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..3f7187c 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")
+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")
+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