Merge topic 'cleanup-gen-lookups'

7ff9ab3b10 Makefile: De-duplicate executable link rule lookup
79f5ef19fe De-duplicate checks for whether a platform uses Windows DLLs
22d3eb5d5e Refactor checks for whether a target has an import library

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !3550

12 files changed, 67 insertions, 64 deletions

diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx
index b366ebb..5a03670 100644
--- a/Source/cmComputeLinkInformation.cxx
+++ b/Source/cmComputeLinkInformation.cxx
@@ -268,10 +268,6 @@ cmComputeLinkInformation::cmComputeLinkInformation(
     return;
   }
 
-  // Check whether we should use an import library for linking a target.
-  this->UseImportLibrary =
-    this->Makefile->IsDefinitionSet("CMAKE_IMPORT_LIBRARY_SUFFIX");
-
   // Check whether we should skip dependencies on shared library files.
   this->LinkDependsNoShared =
     this->Target->GetPropertyAsBool("LINK_DEPENDS_NO_SHARED");
@@ -280,7 +276,7 @@ cmComputeLinkInformation::cmComputeLinkInformation(
   // to use when creating a plugin (module) that obtains symbols from
   // the program that will load it.
   this->LoaderFlag = nullptr;
-  if (!this->UseImportLibrary &&
+  if (!this->Target->IsDLLPlatform() &&
       this->Target->GetType() == cmStateEnums::MODULE_LIBRARY) {
     std::string loader_flag_var = "CMAKE_SHARED_MODULE_LOADER_";
     loader_flag_var += this->LinkLanguage;
@@ -493,9 +489,7 @@ bool cmComputeLinkInformation::Compute()
     std::set<cmGeneratorTarget const*> const& wrongItems =
       cld.GetOldWrongConfigItems();
     for (cmGeneratorTarget const* tgt : wrongItems) {
-      bool implib = (this->UseImportLibrary &&
-                     (tgt->GetType() == cmStateEnums::SHARED_LIBRARY));
-      cmStateEnums::ArtifactType artifact = implib
+      cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(this->Config)
         ? cmStateEnums::ImportLibraryArtifact
         : cmStateEnums::RuntimeBinaryArtifact;
       this->OldLinkDirItems.push_back(
@@ -578,7 +572,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item,
   // Compute the proper name to use to link this library.
   const std::string& config = this->Config;
   bool impexe = (tgt && tgt->IsExecutableWithExports());
-  if (impexe && !this->UseImportLibrary && !this->LoaderFlag) {
+  if (impexe && !tgt->HasImportLibrary(config) && !this->LoaderFlag) {
     // Skip linking to executables on platforms with no import
     // libraries or loader flags.
     return;
@@ -592,7 +586,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item,
       // platform.  Add it now.
       std::string linkItem;
       linkItem = this->LoaderFlag;
-      cmStateEnums::ArtifactType artifact = this->UseImportLibrary
+      cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(config)
         ? cmStateEnums::ImportLibraryArtifact
         : cmStateEnums::RuntimeBinaryArtifact;
 
@@ -616,10 +610,7 @@ void cmComputeLinkInformation::AddItem(std::string const& item,
       // Its object-files should already have been extracted for linking.
     } else {
       // Decide whether to use an import library.
-      bool implib =
-        (this->UseImportLibrary &&
-         (impexe || tgt->GetType() == cmStateEnums::SHARED_LIBRARY));
-      cmStateEnums::ArtifactType artifact = implib
+      cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(config)
         ? cmStateEnums::ImportLibraryArtifact
         : cmStateEnums::RuntimeBinaryArtifact;
 
@@ -694,7 +685,7 @@ void cmComputeLinkInformation::AddSharedDepItem(std::string const& item,
   // linked will be able to find it.
   std::string lib;
   if (tgt) {
-    cmStateEnums::ArtifactType artifact = this->UseImportLibrary
+    cmStateEnums::ArtifactType artifact = tgt->HasImportLibrary(this->Config)
       ? cmStateEnums::ImportLibraryArtifact
       : cmStateEnums::RuntimeBinaryArtifact;
     lib = tgt->GetFullPath(this->Config, artifact);
diff --git a/Source/cmComputeLinkInformation.h b/Source/cmComputeLinkInformation.h
index 863639c..3be2c7f 100644
--- a/Source/cmComputeLinkInformation.h
+++ b/Source/cmComputeLinkInformation.h
@@ -179,7 +179,6 @@ private:
   bool OldLinkDirMode;
   bool OpenBSD;
   bool LinkDependsNoShared;
-  bool UseImportLibrary;
   bool RuntimeUseChrpath;
   bool NoSONameUsesPath;
   bool LinkWithRuntimePath;
diff --git a/Source/cmExportBuildFileGenerator.cxx b/Source/cmExportBuildFileGenerator.cxx
index 5800629..de3e0e2 100644
--- a/Source/cmExportBuildFileGenerator.cxx
+++ b/Source/cmExportBuildFileGenerator.cxx
@@ -236,14 +236,15 @@ void cmExportBuildFileGenerator::SetImportLocationProperty(
     }
 
     // Add the import library for windows DLLs.
-    if (target->HasImportLibrary(config) &&
-        mf->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) {
+    if (target->HasImportLibrary(config)) {
       std::string prop = "IMPORTED_IMPLIB";
       prop += suffix;
       std::string value =
         target->GetFullPath(config, cmStateEnums::ImportLibraryArtifact);
-      target->GetImplibGNUtoMS(config, value, value,
-                               "${CMAKE_IMPORT_LIBRARY_SUFFIX}");
+      if (mf->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) {
+        target->GetImplibGNUtoMS(config, value, value,
+                                 "${CMAKE_IMPORT_LIBRARY_SUFFIX}");
+      }
       properties[prop] = value;
     }
   }
diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx
index 73b4123..7b916cd 100644
--- a/Source/cmFileAPICodemodel.cxx
+++ b/Source/cmFileAPICodemodel.cxx
@@ -1076,17 +1076,16 @@ Json::Value Target::DumpArtifacts()
   }
 
   // Add Windows-specific artifacts produced by the linker.
+  if (this->GT->HasImportLibrary(this->Config)) {
+    Json::Value artifact = Json::objectValue;
+    artifact["path"] =
+      RelativeIfUnder(this->TopBuild,
+                      this->GT->GetFullPath(
+                        this->Config, cmStateEnums::ImportLibraryArtifact));
+    artifacts.append(std::move(artifact)); // NOLINT(*)
+  }
   if (this->GT->IsDLLPlatform() &&
       this->GT->GetType() != cmStateEnums::STATIC_LIBRARY) {
-    if (this->GT->GetType() == cmStateEnums::SHARED_LIBRARY ||
-        this->GT->IsExecutableWithExports()) {
-      Json::Value artifact = Json::objectValue;
-      artifact["path"] =
-        RelativeIfUnder(this->TopBuild,
-                        this->GT->GetFullPath(
-                          this->Config, cmStateEnums::ImportLibraryArtifact));
-      artifacts.append(std::move(artifact)); // NOLINT(*)
-    }
     cmGeneratorTarget::OutputInfo const* output =
       this->GT->GetOutputInfo(this->Config);
     if (output && !output->PdbDir.empty()) {
diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 845937a..c807f6d 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -259,9 +259,6 @@ cmGeneratorTarget::cmGeneratorTarget(cmTarget* t, cmLocalGenerator* lg)
                                      t->GetSourceBacktraces(),
                                      this->SourceEntries, true);
 
-  this->DLLPlatform =
-    !this->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty();
-
   this->PolicyMap = t->GetPolicyMap();
 }
 
@@ -468,7 +465,7 @@ std::string cmGeneratorTarget::GetFilePrefix(
   const std::string& config, cmStateEnums::ArtifactType artifact) const
 {
   if (this->IsImported()) {
-    const char* prefix = this->GetFilePrefixInternal(artifact);
+    const char* prefix = this->GetFilePrefixInternal(config, artifact);
 
     return prefix ? prefix : std::string();
   }
@@ -481,7 +478,7 @@ std::string cmGeneratorTarget::GetFileSuffix(
   const std::string& config, cmStateEnums::ArtifactType artifact) const
 {
   if (this->IsImported()) {
-    const char* suffix = this->GetFileSuffixInternal(artifact);
+    const char* suffix = this->GetFileSuffixInternal(config, artifact);
 
     return suffix ? suffix : std::string();
   }
@@ -508,7 +505,8 @@ std::string cmGeneratorTarget::GetFilePostfix(const std::string& config) const
 }
 
 const char* cmGeneratorTarget::GetFilePrefixInternal(
-  cmStateEnums::ArtifactType artifact, const std::string& language) const
+  std::string const& config, cmStateEnums::ArtifactType artifact,
+  const std::string& language) const
 {
   // no prefix for non-main target types.
   if (this->GetType() != cmStateEnums::STATIC_LIBRARY &&
@@ -523,8 +521,7 @@ const char* cmGeneratorTarget::GetFilePrefixInternal(
 
   // Return an empty prefix for the import library if this platform
   // does not support import libraries.
-  if (isImportedLibraryArtifact &&
-      !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) {
+  if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) {
     return nullptr;
   }
 
@@ -558,7 +555,8 @@ const char* cmGeneratorTarget::GetFilePrefixInternal(
   return targetPrefix;
 }
 const char* cmGeneratorTarget::GetFileSuffixInternal(
-  cmStateEnums::ArtifactType artifact, const std::string& language) const
+  std::string const& config, cmStateEnums::ArtifactType artifact,
+  const std::string& language) const
 {
   // no suffix for non-main target types.
   if (this->GetType() != cmStateEnums::STATIC_LIBRARY &&
@@ -573,8 +571,7 @@ const char* cmGeneratorTarget::GetFileSuffixInternal(
 
   // Return an empty suffix for the import library if this platform
   // does not support import libraries.
-  if (isImportedLibraryArtifact &&
-      !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) {
+  if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) {
     return nullptr;
   }
 
@@ -2357,7 +2354,7 @@ void cmGeneratorTarget::ComputeModuleDefinitionInfo(
 
 bool cmGeneratorTarget::IsDLLPlatform() const
 {
-  return this->DLLPlatform;
+  return this->Target->IsDLLPlatform();
 }
 
 void cmGeneratorTarget::GetAutoUicOptions(std::vector<std::string>& result,
@@ -3924,8 +3921,7 @@ void cmGeneratorTarget::GetFullNameInternal(
 
   // Return an empty name for the import library if this platform
   // does not support import libraries.
-  if (isImportedLibraryArtifact &&
-      !this->Makefile->GetDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX")) {
+  if (isImportedLibraryArtifact && !this->NeedImportLibraryName(config)) {
     outPrefix.clear();
     outBase.clear();
     outSuffix.clear();
@@ -3934,8 +3930,8 @@ void cmGeneratorTarget::GetFullNameInternal(
 
   // retrieve prefix and suffix
   std::string ll = this->GetLinkerLanguage(config);
-  const char* targetPrefix = this->GetFilePrefixInternal(artifact, ll);
-  const char* targetSuffix = this->GetFileSuffixInternal(artifact, ll);
+  const char* targetPrefix = this->GetFilePrefixInternal(config, artifact, ll);
+  const char* targetSuffix = this->GetFileSuffixInternal(config, artifact, ll);
 
   // The implib option is only allowed for shared libraries, module
   // libraries, and executables.
@@ -6363,6 +6359,16 @@ bool cmGeneratorTarget::HasImportLibrary(std::string const& config) const
           this->GetManagedType(config) != ManagedType::Managed);
 }
 
+bool cmGeneratorTarget::NeedImportLibraryName(std::string const& config) const
+{
+  return this->HasImportLibrary(config) ||
+    // On DLL platforms we always generate the import library name
+    // just in case the sources have export markup.
+    (this->IsDLLPlatform() &&
+     (this->GetType() == cmStateEnums::EXECUTABLE ||
+      this->GetType() == cmStateEnums::MODULE_LIBRARY));
+}
+
 std::string cmGeneratorTarget::GetSupportDirectory() const
 {
   std::string dir = this->LocalGenerator->GetCurrentBinaryDirectory();
diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h
index 627a055..e86535d 100644
--- a/Source/cmGeneratorTarget.h
+++ b/Source/cmGeneratorTarget.h
@@ -743,9 +743,13 @@ private:
 
   mutable std::map<std::string, bool> DebugCompatiblePropertiesDone;
 
-  const char* GetFilePrefixInternal(cmStateEnums::ArtifactType artifact,
+  bool NeedImportLibraryName(std::string const& config) const;
+
+  const char* GetFilePrefixInternal(std::string const& config,
+                                    cmStateEnums::ArtifactType artifact,
                                     const std::string& language = "") const;
-  const char* GetFileSuffixInternal(cmStateEnums::ArtifactType artifact,
+  const char* GetFileSuffixInternal(std::string const& config,
+                                    cmStateEnums::ArtifactType artifact,
                                     const std::string& language = "") const;
 
   std::string GetFullNameInternal(const std::string& config,
@@ -909,7 +913,6 @@ private:
   mutable bool DebugSourcesDone;
   mutable bool LinkImplementationLanguageIsContextDependent;
   mutable bool UtilityItemsDone;
-  bool DLLPlatform;
 
   bool ComputePDBOutputDir(const std::string& kind, const std::string& config,
                            std::string& out) const;
diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx
index dba4bbb..750ed8c 100644
--- a/Source/cmInstallCommand.cxx
+++ b/Source/cmInstallCommand.cxx
@@ -389,10 +389,6 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
     return true;
   }
 
-  // Check whether this is a DLL platform.
-  bool dll_platform =
-    !this->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty();
-
   for (std::string const& tgt : targetList) {
 
     if (this->Makefile->IsAlias(tgt)) {
@@ -472,7 +468,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
         // Shared libraries are handled differently on DLL and non-DLL
         // platforms.  All windows platforms are DLL platforms including
         // cygwin.  Currently no other platform is a DLL platform.
-        if (dll_platform) {
+        if (target.IsDLLPlatform()) {
           // When in namelink only mode skip all libraries on Windows.
           if (namelinkMode == cmInstallTargetGenerator::NamelinkModeOnly) {
             continue;
@@ -641,7 +637,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
         // On DLL platforms an executable may also have an import
         // library.  Install it to the archive destination if it
         // exists.
-        if (dll_platform && !archiveArgs.GetDestination().empty() &&
+        if (target.IsDLLPlatform() && !archiveArgs.GetDestination().empty() &&
             target.IsExecutableWithExports()) {
           // The import library uses the ARCHIVE properties.
           archiveGenerator = CreateInstallTargetGenerator(
diff --git a/Source/cmJsonObjects.cxx b/Source/cmJsonObjects.cxx
index 505188f..2423faf 100644
--- a/Source/cmJsonObjects.cxx
+++ b/Source/cmJsonObjects.cxx
@@ -516,9 +516,11 @@ static Json::Value DumpTarget(cmGeneratorTarget* target,
     Json::Value artifacts = Json::arrayValue;
     artifacts.append(
       target->GetFullPath(config, cmStateEnums::RuntimeBinaryArtifact));
-    if (target->IsDLLPlatform()) {
+    if (target->HasImportLibrary(config)) {
       artifacts.append(
         target->GetFullPath(config, cmStateEnums::ImportLibraryArtifact));
+    }
+    if (target->IsDLLPlatform()) {
       const cmGeneratorTarget::OutputInfo* output =
         target->GetOutputInfo(config);
       if (output && !output->PdbDir.empty()) {
diff --git a/Source/cmLinkLineComputer.cxx b/Source/cmLinkLineComputer.cxx
index 469faca..8746b35 100644
--- a/Source/cmLinkLineComputer.cxx
+++ b/Source/cmLinkLineComputer.cxx
@@ -110,8 +110,7 @@ std::string cmLinkLineComputer::ComputeLinkPath(
       if (target->GetType() == cmStateEnums::STATIC_LIBRARY ||
           target->GetType() == cmStateEnums::SHARED_LIBRARY) {
         cmStateEnums::ArtifactType type = cmStateEnums::RuntimeBinaryArtifact;
-        if (target->GetType() == cmStateEnums::SHARED_LIBRARY &&
-            target->IsDLLPlatform()) {
+        if (target->HasImportLibrary(cli.GetConfig())) {
           type = cmStateEnums::ImportLibraryArtifact;
         }
 
diff --git a/Source/cmMakefileExecutableTargetGenerator.cxx b/Source/cmMakefileExecutableTargetGenerator.cxx
index dcee47e..a7f2a97 100644
--- a/Source/cmMakefileExecutableTargetGenerator.cxx
+++ b/Source/cmMakefileExecutableTargetGenerator.cxx
@@ -483,9 +483,8 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
 
   // Construct the main link rule.
   std::vector<std::string> real_link_commands;
-  std::string linkRuleVar = "CMAKE_";
-  linkRuleVar += linkLanguage;
-  linkRuleVar += "_LINK_EXECUTABLE";
+  std::string linkRuleVar = this->GeneratorTarget->GetCreateRuleVariable(
+    linkLanguage, this->ConfigName);
   std::string linkRule = this->GetLinkRule(linkRuleVar);
   std::vector<std::string> commands1;
   cmSystemTools::ExpandListArgument(linkRule, real_link_commands);
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 28d8c64..773487e 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -170,7 +170,7 @@ public:
   cmPropertyMap Properties;
   bool IsGeneratorProvided;
   bool HaveInstallRule;
-  bool DLLPlatform;
+  bool IsDLLPlatform;
   bool IsAndroid;
   bool IsImportedTarget;
   bool ImportedGloballyVisible;
@@ -218,7 +218,7 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type,
   impl->Name = name;
   impl->IsGeneratorProvided = false;
   impl->HaveInstallRule = false;
-  impl->DLLPlatform = false;
+  impl->IsDLLPlatform = false;
   impl->IsAndroid = false;
   impl->IsImportedTarget =
     (vis == VisibilityImported || vis == VisibilityImportedGlobally);
@@ -226,7 +226,7 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type,
   impl->BuildInterfaceIncludesAppended = false;
 
   // Check whether this is a DLL platform.
-  impl->DLLPlatform =
+  impl->IsDLLPlatform =
     !impl->Makefile->GetSafeDefinition("CMAKE_IMPORT_LIBRARY_SUFFIX").empty();
 
   // Check whether we are targeting an Android platform.
@@ -1659,6 +1659,11 @@ cmPropertyMap const& cmTarget::GetProperties() const
   return impl->Properties;
 }
 
+bool cmTarget::IsDLLPlatform() const
+{
+  return impl->IsDLLPlatform;
+}
+
 bool cmTarget::IsImported() const
 {
   return impl->IsImportedTarget;
@@ -1874,7 +1879,7 @@ bool cmTarget::GetMappedConfig(std::string const& desired_config,
   // If we needed to find one of the mapped configurations but did not
   // On a DLL platform there may be only IMPORTED_IMPLIB for a shared
   // library or an executable with exports.
-  bool allowImp = (impl->DLLPlatform &&
+  bool allowImp = (this->IsDLLPlatform() &&
                    (this->GetType() == cmStateEnums::SHARED_LIBRARY ||
                     this->IsExecutableWithExports()));
 
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index f47cc14..d9e87b8 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -177,6 +177,9 @@ public:
   //! Get all properties
   cmPropertyMap const& GetProperties() const;
 
+  //! Return whether or not the target is for a DLL platform.
+  bool IsDLLPlatform() const;
+
   bool IsImported() const;
   bool IsImportedGloballyVisible() const;