Merge topic 'refactor-target-construction'

7a2e114d cmTarget: Inline SetType method at only remaining call site
00e78c19 cmTarget: Construct with basic information up front
9d11bd50 Avoid requiring default cmTarget constructor for map indexing
d97513d8 cmTarget: Add method to get a copy adapted for a directory

6 files changed, 114 insertions, 78 deletions

diff --git a/Source/cmExportTryCompileFileGenerator.cxx b/Source/cmExportTryCompileFileGenerator.cxx
index a0aefb8..6c31481 100644
--- a/Source/cmExportTryCompileFileGenerator.cxx
+++ b/Source/cmExportTryCompileFileGenerator.cxx
@@ -77,9 +77,8 @@ std::string cmExportTryCompileFileGenerator::FindTargets(
 
   CM_AUTO_PTR<cmCompiledGeneratorExpression> cge = ge.Parse(prop);
 
-  cmTarget dummyHead;
-  dummyHead.SetType(cmState::EXECUTABLE, "try_compile_dummy_exe");
-  dummyHead.SetMakefile(tgt->Target->GetMakefile());
+  cmTarget dummyHead("try_compile_dummy_exe", cmState::EXECUTABLE,
+                     cmTarget::VisibilityNormal, tgt->Target->GetMakefile());
 
   cmGeneratorTarget gDummyHead(&dummyHead, tgt->GetLocalGenerator());
 
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index ef8266f..e85d80e 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -1068,8 +1068,8 @@ void cmGlobalGenerator::Configure()
     cmTargets* targets = &(mf->GetTargets());
     cmTargets::iterator tit;
     for (tit = globalTargets.begin(); tit != globalTargets.end(); ++tit) {
-      (*targets)[tit->first] = tit->second;
-      (*targets)[tit->first].SetMakefile(mf);
+      targets->insert(
+        cmTargets::value_type(tit->first, tit->second.CopyForDirectory(mf)));
     }
   }
 
@@ -2101,9 +2101,12 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     }
   }
   if (cmSystemTools::FileExists(configFile.c_str())) {
-    (*targets)[this->GetPackageTargetName()] = this->CreateGlobalTarget(
-      this->GetPackageTargetName(), "Run CPack packaging tool...",
-      &cpackCommandLines, depends, workingDir.c_str(), /*uses_terminal*/ true);
+    targets->insert(cmTargets::value_type(
+      this->GetPackageTargetName(),
+      this->CreateGlobalTarget(this->GetPackageTargetName(),
+                               "Run CPack packaging tool...",
+                               &cpackCommandLines, depends, workingDir.c_str(),
+                               /*uses_terminal*/ true)));
   }
   // CPack source
   const char* packageSourceTargetName = this->GetPackageSourceTargetName();
@@ -2122,10 +2125,12 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     if (cmSystemTools::FileExists(configFile.c_str())) {
       singleLine.push_back(configFile);
       cpackCommandLines.push_back(singleLine);
-      (*targets)[packageSourceTargetName] = this->CreateGlobalTarget(
-        packageSourceTargetName, "Run CPack packaging tool for source...",
-        &cpackCommandLines, depends, workingDir.c_str(),
-        /*uses_terminal*/ true);
+      targets->insert(cmTargets::value_type(
+        packageSourceTargetName,
+        this->CreateGlobalTarget(
+          packageSourceTargetName, "Run CPack packaging tool for source...",
+          &cpackCommandLines, depends, workingDir.c_str(),
+          /*uses_terminal*/ true)));
     }
   }
 
@@ -2146,10 +2151,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
       singleLine.push_back("$(ARGS)");
     }
     cpackCommandLines.push_back(singleLine);
-    (*targets)[this->GetTestTargetName()] =
+    targets->insert(cmTargets::value_type(
+      this->GetTestTargetName(),
       this->CreateGlobalTarget(this->GetTestTargetName(), "Running tests...",
                                &cpackCommandLines, depends, CM_NULLPTR,
-                               /*uses_terminal*/ true);
+                               /*uses_terminal*/ true)));
   }
 
   // Edit Cache
@@ -2167,18 +2173,22 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
       singleLine.push_back("-H$(CMAKE_SOURCE_DIR)");
       singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
       cpackCommandLines.push_back(singleLine);
-      (*targets)[editCacheTargetName] = this->CreateGlobalTarget(
-        editCacheTargetName, "Running CMake cache editor...",
-        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true);
+      targets->insert(cmTargets::value_type(
+        editCacheTargetName,
+        this->CreateGlobalTarget(
+          editCacheTargetName, "Running CMake cache editor...",
+          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
     } else {
       singleLine.push_back(cmSystemTools::GetCMakeCommand());
       singleLine.push_back("-E");
       singleLine.push_back("echo");
       singleLine.push_back("No interactive CMake dialog available.");
       cpackCommandLines.push_back(singleLine);
-      (*targets)[editCacheTargetName] = this->CreateGlobalTarget(
-        editCacheTargetName, "No interactive CMake dialog available...",
-        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ false);
+      targets->insert(cmTargets::value_type(
+        editCacheTargetName,
+        this->CreateGlobalTarget(
+          editCacheTargetName, "No interactive CMake dialog available...",
+          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ false)));
     }
   }
 
@@ -2193,9 +2203,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     singleLine.push_back("-H$(CMAKE_SOURCE_DIR)");
     singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
     cpackCommandLines.push_back(singleLine);
-    (*targets)[rebuildCacheTargetName] = this->CreateGlobalTarget(
-      rebuildCacheTargetName, "Running CMake to regenerate build system...",
-      &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true);
+    targets->insert(cmTargets::value_type(
+      rebuildCacheTargetName,
+      this->CreateGlobalTarget(
+        rebuildCacheTargetName, "Running CMake to regenerate build system...",
+        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
   }
 
   // Install
@@ -2219,9 +2231,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
         ostr << "Only default component available";
       }
       singleLine.push_back(ostr.str());
-      (*targets)["list_install_components"] = this->CreateGlobalTarget(
-        "list_install_components", ostr.str().c_str(), &cpackCommandLines,
-        depends, CM_NULLPTR, /*uses_terminal*/ false);
+      targets->insert(cmTargets::value_type(
+        "list_install_components",
+        this->CreateGlobalTarget("list_install_components", ostr.str().c_str(),
+                                 &cpackCommandLines, depends, CM_NULLPTR,
+                                 /*uses_terminal*/ false)));
     }
     std::string cmd = cmSystemTools::GetCMakeCommand();
     cpackCommandLines.erase(cpackCommandLines.begin(),
@@ -2260,9 +2274,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     singleLine.push_back("-P");
     singleLine.push_back("cmake_install.cmake");
     cpackCommandLines.push_back(singleLine);
-    (*targets)[this->GetInstallTargetName()] = this->CreateGlobalTarget(
-      this->GetInstallTargetName(), "Install the project...",
-      &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true);
+    targets->insert(cmTargets::value_type(
+      this->GetInstallTargetName(),
+      this->CreateGlobalTarget(this->GetInstallTargetName(),
+                               "Install the project...", &cpackCommandLines,
+                               depends, CM_NULLPTR, /*uses_terminal*/ true)));
 
     // install_local
     if (const char* install_local = this->GetInstallLocalTargetName()) {
@@ -2274,9 +2290,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
                               cpackCommandLines.end());
       cpackCommandLines.push_back(localCmdLine);
 
-      (*targets)[install_local] = this->CreateGlobalTarget(
-        install_local, "Installing only the local directory...",
-        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true);
+      targets->insert(cmTargets::value_type(
+        install_local,
+        this->CreateGlobalTarget(
+          install_local, "Installing only the local directory...",
+          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
     }
 
     // install_strip
@@ -2290,9 +2308,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
                               cpackCommandLines.end());
       cpackCommandLines.push_back(stripCmdLine);
 
-      (*targets)[install_strip] = this->CreateGlobalTarget(
-        install_strip, "Installing the project stripped...",
-        &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true);
+      targets->insert(cmTargets::value_type(
+        install_strip,
+        this->CreateGlobalTarget(
+          install_strip, "Installing the project stripped...",
+          &cpackCommandLines, depends, CM_NULLPTR, /*uses_terminal*/ true)));
     }
   }
 }
@@ -2332,8 +2352,8 @@ cmTarget cmGlobalGenerator::CreateGlobalTarget(
   const char* workingDirectory, bool uses_terminal)
 {
   // Package
-  cmTarget target;
-  target.SetType(cmState::GLOBAL_TARGET, name);
+  cmTarget target(name, cmState::GLOBAL_TARGET, cmTarget::VisibilityNormal,
+                  CM_NULLPTR);
   target.SetProperty("EXCLUDE_FROM_ALL", "TRUE");
 
   std::vector<std::string> no_outputs;
diff --git a/Source/cmInstallTargetsCommand.cxx b/Source/cmInstallTargetsCommand.cxx
index 056ea24..6b594b6 100644
--- a/Source/cmInstallTargetsCommand.cxx
+++ b/Source/cmInstallTargetsCommand.cxx
@@ -37,14 +37,17 @@ bool cmInstallTargetsCommand::InitialPass(std::vector<std::string> const& args,
       }
 
       runtime_dir = *s;
-    } else if (tgts.find(*s) != tgts.end()) {
-      tgts[*s].SetInstallPath(args[0].c_str());
-      tgts[*s].SetRuntimeInstallPath(runtime_dir.c_str());
-      tgts[*s].SetHaveInstallRule(true);
     } else {
-      std::string str = "Cannot find target: \"" + *s + "\" to install.";
-      this->SetError(str);
-      return false;
+      cmTargets::iterator ti = tgts.find(*s);
+      if (ti != tgts.end()) {
+        ti->second.SetInstallPath(args[0].c_str());
+        ti->second.SetRuntimeInstallPath(runtime_dir.c_str());
+        ti->second.SetHaveInstallRule(true);
+      } else {
+        std::string str = "Cannot find target: \"" + *s + "\" to install.";
+        this->SetError(str);
+        return false;
+      }
     }
   }
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index e5a5e6e..508c670 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -979,8 +979,9 @@ void cmMakefile::AddCustomCommandOldStyle(
     // then add the source to the target to make sure the rule is
     // included.
     if (sf && !sf->GetPropertyAsBool("__CMAKE_RULE")) {
-      if (this->Targets.find(target) != this->Targets.end()) {
-        this->Targets[target].AddSource(sf->GetFullPath());
+      cmTargets::iterator ti = this->Targets.find(target);
+      if (ti != this->Targets.end()) {
+        ti->second.AddSource(sf->GetFullPath());
       } else {
         cmSystemTools::Error("Attempt to add a custom rule to a target "
                              "that does not exist yet for target ",
@@ -1924,10 +1925,10 @@ cmTarget* cmMakefile::AddNewTarget(cmState::TargetType type,
                                    const std::string& name)
 {
   cmTargets::iterator it =
-    this->Targets.insert(cmTargets::value_type(name, cmTarget())).first;
-  cmTarget& target = it->second;
-  target.SetType(type, name);
-  target.SetMakefile(this);
+    this->Targets
+      .insert(cmTargets::value_type(
+        name, cmTarget(name, type, cmTarget::VisibilityNormal, this)))
+      .first;
   this->GetGlobalGenerator()->IndexTarget(&it->second);
   return &it->second;
 }
@@ -3709,10 +3710,10 @@ cmTarget* cmMakefile::AddImportedTarget(const std::string& name,
                                         cmState::TargetType type, bool global)
 {
   // Create the target.
-  CM_AUTO_PTR<cmTarget> target(new cmTarget);
-  target->SetType(type, name);
-  target->MarkAsImported(global);
-  target->SetMakefile(this);
+  CM_AUTO_PTR<cmTarget> target(
+    new cmTarget(name, type, global ? cmTarget::VisibilityImportedGlobally
+                                    : cmTarget::VisibilityImported,
+                 this));
 
   // Add to the set of available imported targets.
   this->ImportedTargets[name] = target.get();
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index d5274cd..d964f00 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -59,28 +59,41 @@ public:
   std::vector<cmListFileBacktrace> LinkImplementationPropertyBacktraces;
 };
 
-cmTarget::cmTarget()
+cmTarget::cmTarget(std::string const& name, cmState::TargetType type,
+                   Visibility vis, cmMakefile* mf)
 {
+  assert(mf || type == cmState::GLOBAL_TARGET);
+  this->Name = name;
+  this->TargetTypeValue = type;
   this->Makefile = CM_NULLPTR;
   this->HaveInstallRule = false;
   this->DLLPlatform = false;
   this->IsAndroid = false;
-  this->IsImportedTarget = false;
-  this->ImportedGloballyVisible = false;
+  this->IsImportedTarget =
+    (vis == VisibilityImported || vis == VisibilityImportedGlobally);
+  this->ImportedGloballyVisible = vis == VisibilityImportedGlobally;
   this->BuildInterfaceIncludesAppended = false;
-}
 
-void cmTarget::SetType(cmState::TargetType type, const std::string& name)
-{
-  this->Name = name;
   // only add dependency information for library targets
-  this->TargetTypeValue = type;
   if (this->TargetTypeValue >= cmState::STATIC_LIBRARY &&
       this->TargetTypeValue <= cmState::MODULE_LIBRARY) {
     this->RecordDependencies = true;
   } else {
     this->RecordDependencies = false;
   }
+
+  if (mf) {
+    this->SetMakefile(mf);
+  }
+}
+
+cmTarget cmTarget::CopyForDirectory(cmMakefile* mf) const
+{
+  assert(this->GetType() == cmState::GLOBAL_TARGET);
+  assert(this->GetMakefile() == CM_NULLPTR);
+  cmTarget result(*this);
+  result.SetMakefile(mf);
+  return result;
 }
 
 void cmTarget::SetMakefile(cmMakefile* mf)
@@ -1062,12 +1075,6 @@ void cmTarget::CheckProperty(const std::string& prop,
   }
 }
 
-void cmTarget::MarkAsImported(bool global)
-{
-  this->IsImportedTarget = true;
-  this->ImportedGloballyVisible = global;
-}
-
 bool cmTarget::HandleLocationPropertyPolicy(cmMakefile* context) const
 {
   if (this->IsImported()) {
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index fc30166..ebc92f3 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -63,7 +63,16 @@ private:
 class cmTarget
 {
 public:
-  cmTarget();
+  enum Visibility
+  {
+    VisibilityNormal,
+    VisibilityImported,
+    VisibilityImportedGlobally
+  };
+
+  cmTarget(std::string const& name, cmState::TargetType type, Visibility vis,
+           cmMakefile* mf);
+
   enum CustomCommandType
   {
     PRE_BUILD,
@@ -76,18 +85,13 @@ public:
    */
   cmState::TargetType GetType() const { return this->TargetTypeValue; }
 
-  /**
-   * Set the target type
-   */
-  void SetType(cmState::TargetType f, const std::string& name);
-
-  void MarkAsImported(bool global = false);
-
   ///! Set/Get the name of the target
   const std::string& GetName() const { return this->Name; }
 
-  ///! Set the cmMakefile that owns this target
-  void SetMakefile(cmMakefile* mf);
+  /** Get a copy of this target adapted for the given directory.  */
+  cmTarget CopyForDirectory(cmMakefile* mf) const;
+
+  /** Get the cmMakefile that owns this target.  */
   cmMakefile* GetMakefile() const { return this->Makefile; }
 
 #define DECLARE_TARGET_POLICY(POLICY)                                         \
@@ -279,6 +283,8 @@ public:
   };
 
 private:
+  void SetMakefile(cmMakefile* mf);
+
   bool HandleLocationPropertyPolicy(cmMakefile* context) const;
 
   const char* GetSuffixVariableInternal(bool implib) const;