Merge topic 'OUTPUT_NAME-genex-no-recursion'

3c37d264 cmGeneratorTarget: Avoid recursion in GetOutputName method

Source/cmGeneratorTarget.cxx

@@ -270,48 +270,71 @@ const char *cmGeneratorTarget::GetProperty(const std::string& prop) const
 std::string cmGeneratorTarget::GetOutputName(const std::string& config,
                                              bool implib) const
 {
-  std::vector<std::string> props;
-  std::string type = this->Target->GetOutputTargetType(implib);
-  std::string configUpper = cmSystemTools::UpperCase(config);
-  if(!type.empty() && !configUpper.empty())
-    {
-    // <ARCHIVE|LIBRARY|RUNTIME>_OUTPUT_NAME_<CONFIG>
-    props.push_back(type + "_OUTPUT_NAME_" + configUpper);
-    }
-  if(!type.empty())
+  // Lookup/compute/cache the output name for this configuration.
+  OutputNameKey key(config, implib);
+  cmGeneratorTarget::OutputNameMapType::iterator i =
+    this->OutputNameMap.find(key);
+  if(i == this->OutputNameMap.end())
     {
-    // <ARCHIVE|LIBRARY|RUNTIME>_OUTPUT_NAME
-    props.push_back(type + "_OUTPUT_NAME");
-    }
-  if(!configUpper.empty())
-    {
-    // OUTPUT_NAME_<CONFIG>
-    props.push_back("OUTPUT_NAME_" + configUpper);
-    // <CONFIG>_OUTPUT_NAME
-    props.push_back(configUpper + "_OUTPUT_NAME");
-    }
-  // OUTPUT_NAME
-  props.push_back("OUTPUT_NAME");
+    // Add empty name in map to detect potential recursion.
+    OutputNameMapType::value_type entry(key, "");
+    i = this->OutputNameMap.insert(entry).first;
 
-  std::string outName;
-  for(std::vector<std::string>::const_iterator i = props.begin();
-      i != props.end(); ++i)
-    {
-    if (const char* outNameProp = this->Target->GetProperty(*i))
+    // Compute output name.
+    std::vector<std::string> props;
+    std::string type = this->Target->GetOutputTargetType(implib);
+    std::string configUpper = cmSystemTools::UpperCase(config);
+    if(!type.empty() && !configUpper.empty())
       {
-      outName = outNameProp;
-      break;
+      // <ARCHIVE|LIBRARY|RUNTIME>_OUTPUT_NAME_<CONFIG>
+      props.push_back(type + "_OUTPUT_NAME_" + configUpper);
+      }
+    if(!type.empty())
+      {
+      // <ARCHIVE|LIBRARY|RUNTIME>_OUTPUT_NAME
+      props.push_back(type + "_OUTPUT_NAME");
+      }
+    if(!configUpper.empty())
+      {
+      // OUTPUT_NAME_<CONFIG>
+      props.push_back("OUTPUT_NAME_" + configUpper);
+      // <CONFIG>_OUTPUT_NAME
+      props.push_back(configUpper + "_OUTPUT_NAME");
+      }
+    // OUTPUT_NAME
+    props.push_back("OUTPUT_NAME");
+
+    std::string outName;
+    for(std::vector<std::string>::const_iterator it = props.begin();
+        it != props.end(); ++it)
+      {
+      if (const char* outNameProp = this->Target->GetProperty(*it))
+        {
+        outName = outNameProp;
+        break;
+        }
       }
-    }
 
-  if (outName.empty())
+    if(outName.empty())
+      {
+      outName = this->GetName();
+      }
+
+    // Now evaluate genex and update the previously-prepared map entry.
+    cmGeneratorExpression ge;
+    cmsys::auto_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(outName);
+    i->second = cge->Evaluate(this->Makefile, config);
+    }
+  else if(i->second.empty())
     {
-    outName = this->GetName();
+    // An empty map entry indicates we have been called recursively
+    // from the above block.
+    this->Makefile->GetCMakeInstance()->IssueMessage(
+      cmake::FATAL_ERROR,
+      "Target '" + this->GetName() + "' OUTPUT_NAME depends on itself.",
+      this->Target->GetBacktrace());
     }
-
-  cmGeneratorExpression ge;
-  cmsys::auto_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(outName);
-  return cge->Evaluate(this->Makefile, config);
+  return i->second;
 }
 
 //----------------------------------------------------------------------------

Source/cmGeneratorTarget.h

@@ -375,6 +375,10 @@ private:
   };
   mutable std::map<std::string, LinkImplClosure> LinkImplClosureMap;
 
+  typedef std::pair<std::string, bool> OutputNameKey;
+  typedef std::map<OutputNameKey, std::string> OutputNameMapType;
+  mutable OutputNameMapType OutputNameMap;
+
 public:
   std::vector<cmTarget const*> const&
     GetLinkImplementationClosure(const std::string& config) const;

Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at OUTPUT_NAME-recursion.cmake:[0-9]+ \(add_executable\):
+  Target 'empty1' OUTPUT_NAME depends on itself.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion.cmake

@@ -0,0 +1,3 @@
+enable_language(C)
+add_executable(empty1 empty.c)
+set_property(TARGET empty1 PROPERTY OUTPUT_NAME $<TARGET_FILE_NAME:empty1>)

Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake

@@ -26,6 +26,7 @@ run_cmake(COMPILE_LANGUAGE-add_library)
 run_cmake(COMPILE_LANGUAGE-add_test)
 run_cmake(COMPILE_LANGUAGE-unknown-lang)
 run_cmake(TARGET_FILE-recursion)
+run_cmake(OUTPUT_NAME-recursion)
 
 run_cmake(ImportedTarget-TARGET_PDB_FILE)
 if(LINKER_SUPPORTS_PDB)

Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake

@@ -1,3 +1,4 @@
 enable_language(C)
 add_executable(empty1 empty.c)
+set_property(TARGET empty1 PROPERTY OUTPUT_NAME $<TARGET_FILE_NAME:empty1>)
 set_property(TARGET empty1 PROPERTY RUNTIME_OUTPUT_DIRECTORY $<TARGET_FILE_DIR:empty1>)