Merge topic 'vs-flag-table-refactor'

1b33150f7e cmGlobalVisualStudio10Generator: Generalize flag table lookup interface
414b5e0119 cmGlobalVisualStudio10Generator: Clarify LoadFlagTable argument name
cbba9f26a8 cmGlobalVisualStudio10Generator: Drop unnecessary temporaries
d5522f096d cmGlobalVisualStudio10Generator: Drop default toolset name canonicalization
99f6f1b3b9 cmGlobalVisualStudio10Generator: Remove redundant arguments
1b774e18fb cmGlobalVisualStudio10Generator: Remove redundant arguments
a1c0758ea0 cmGlobalVisualStudio10Generator: Clarify method name
e562e8987c cmGlobalVisualStudio10Generator: Remove unused flag table method argument
...

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

Source/CMakeLists.txt

@@ -785,8 +785,6 @@ if (WIN32)
       cmVisualStudioGeneratorOptions.cxx
       cmVisualStudio10TargetGenerator.h
       cmVisualStudio10TargetGenerator.cxx
-      cmVisualStudio10ToolsetOptions.h
-      cmVisualStudio10ToolsetOptions.cxx
       cmLocalVisualStudio10Generator.cxx
       cmLocalVisualStudio10Generator.h
       cmGlobalVisualStudio10Generator.h

Source/cmGlobalVisualStudio10Generator.cxx

@@ -1358,137 +1358,248 @@ static cmIDEFlagTable const* cmLoadFlagTableJson(
   return ret;
 }
 
-static std::string cmGetFlagTableName(std::string const& toolsetName,
-                                      std::string const& table)
+cm::optional<std::string> cmGlobalVisualStudio10Generator::FindFlagTable(
+  cm::string_view toolsetName, cm::string_view table) const
 {
-  return cmSystemTools::GetCMakeRoot() + "/Templates/MSBuild/FlagTables/" +
-    toolsetName + "_" + table + ".json";
+  std::string fullPath =
+    cmStrCat(cmSystemTools::GetCMakeRoot(), "/Templates/MSBuild/FlagTables/",
+             toolsetName, '_', table, ".json");
+  if (cmSystemTools::FileExists(fullPath)) {
+    return fullPath;
+  }
+  return {};
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::LoadFlagTable(
-  std::string const& optionsName, std::string const& toolsetName,
-  std::string const& defaultName, std::string const& table) const
+  std::string const& toolSpecificName, std::string const& defaultName,
+  std::string const& table) const
 {
-  cmIDEFlagTable const* ret = nullptr;
+  cmMakefile* mf = this->GetCurrentMakefile();
 
   std::string filename;
-  if (!optionsName.empty()) {
-    filename = cmGetFlagTableName(optionsName, table);
-    ret = cmLoadFlagTableJson(filename);
-  } else {
-    filename = cmGetFlagTableName(toolsetName, table);
-    if (cmSystemTools::FileExists(filename)) {
-      ret = cmLoadFlagTableJson(filename);
+  if (!toolSpecificName.empty()) {
+    if (cm::optional<std::string> found =
+          this->FindFlagTable(toolSpecificName, table)) {
+      filename = std::move(*found);
     } else {
-      filename = cmGetFlagTableName(defaultName, table);
-      ret = cmLoadFlagTableJson(filename);
+      mf->IssueMessage(MessageType::FATAL_ERROR,
+                       cmStrCat("JSON flag table for ", table,
+                                " not found for toolset ", toolSpecificName));
+      return nullptr;
+    }
+  } else {
+    std::string const& genericName =
+      this->CanonicalToolsetName(this->GetPlatformToolsetString());
+    cm::optional<std::string> found = this->FindFlagTable(genericName, table);
+    if (!found) {
+      found = this->FindFlagTable(defaultName, table);
+    }
+    if (found) {
+      filename = std::move(*found);
+    } else {
+      mf->IssueMessage(MessageType::FATAL_ERROR,
+                       cmStrCat("JSON flag table for ", table,
+                                " not found for toolset ", genericName, " ",
+                                defaultName));
+      return nullptr;
     }
   }
 
-  if (!ret) {
-    cmMakefile* mf = this->GetCurrentMakefile();
-
-    std::ostringstream e;
-    /* clang-format off */
-    e << "JSON flag table \"" << filename <<
-      "\" could not be loaded.\n";
-    /* clang-format on */
-    mf->IssueMessage(MessageType::FATAL_ERROR, e.str());
+  if (cmIDEFlagTable const* ret = cmLoadFlagTableJson(filename)) {
+    return ret;
   }
-  return ret;
+
+  mf->IssueMessage(
+    MessageType::FATAL_ERROR,
+    cmStrCat("JSON flag table could not be loaded:\n  ", filename));
+  return nullptr;
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetClFlagTable() const
 {
-  std::string optionsName = this->ToolsetOptions.GetClFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultCLFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "CL");
+  return LoadFlagTable(this->GetClFlagTableName(),
+                       this->DefaultCLFlagTableName, "CL");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetCSharpFlagTable()
   const
 {
-  std::string optionsName = this->ToolsetOptions.GetCSharpFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultCSharpFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "CSharp");
+  return LoadFlagTable(this->GetCSharpFlagTableName(),
+                       this->DefaultCSharpFlagTableName, "CSharp");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetRcFlagTable() const
 {
-  std::string optionsName = this->ToolsetOptions.GetRcFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultRCFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "RC");
+  return LoadFlagTable(this->GetRcFlagTableName(),
+                       this->DefaultRCFlagTableName, "RC");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetLibFlagTable() const
 {
-  std::string optionsName = this->ToolsetOptions.GetLibFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultLibFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "LIB");
+  return LoadFlagTable(this->GetLibFlagTableName(),
+                       this->DefaultLibFlagTableName, "LIB");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetLinkFlagTable() const
 {
-  std::string optionsName = this->ToolsetOptions.GetLinkFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultLinkFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "Link");
+  return LoadFlagTable(this->GetLinkFlagTableName(),
+                       this->DefaultLinkFlagTableName, "Link");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetCudaFlagTable() const
 {
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultCudaFlagTableName);
-  return LoadFlagTable("", toolsetName, defaultName, "Cuda");
+  return LoadFlagTable(std::string(), this->DefaultCudaFlagTableName, "Cuda");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetCudaHostFlagTable()
   const
 {
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultCudaHostFlagTableName);
-  return LoadFlagTable("", toolsetName, defaultName, "CudaHost");
+  return LoadFlagTable(std::string(), this->DefaultCudaHostFlagTableName,
+                       "CudaHost");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetMasmFlagTable() const
 {
-  std::string optionsName = this->ToolsetOptions.GetMasmFlagTableName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultMasmFlagTableName);
-  return LoadFlagTable(optionsName, toolsetName, defaultName, "MASM");
+  return LoadFlagTable(this->GetMasmFlagTableName(),
+                       this->DefaultMasmFlagTableName, "MASM");
 }
 
 cmIDEFlagTable const* cmGlobalVisualStudio10Generator::GetNasmFlagTable() const
 {
-  std::string toolsetName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->GetPlatformToolsetString());
-  std::string defaultName = this->ToolsetOptions.GetToolsetName(
-    this->GetPlatformName(), this->DefaultNasmFlagTableName);
-  return LoadFlagTable("", toolsetName, defaultName, "NASM");
+  return LoadFlagTable(std::string(), this->DefaultNasmFlagTableName, "NASM");
+}
+
+std::string cmGlobalVisualStudio10Generator::GetClFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if (toolset == "v142") {
+    return "v142";
+  } else if (toolset == "v141") {
+    return "v141";
+  } else if (useToolset == "v140") {
+    return "v140";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::GetCSharpFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if (useToolset == "v142") {
+    return "v142";
+  } else if (useToolset == "v141") {
+    return "v141";
+  } else if (useToolset == "v140") {
+    return "v140";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::GetRcFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if ((useToolset == "v140") || (useToolset == "v141") ||
+      (useToolset == "v142")) {
+    return "v14";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::GetLibFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if ((useToolset == "v140") || (useToolset == "v141") ||
+      (useToolset == "v142")) {
+    return "v14";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::GetLinkFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if (useToolset == "v142") {
+    return "v142";
+  } else if (useToolset == "v141") {
+    return "v141";
+  } else if (useToolset == "v140") {
+    return "v140";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::GetMasmFlagTableName() const
+{
+  std::string const& toolset = this->GetPlatformToolsetString();
+  std::string const useToolset = this->CanonicalToolsetName(toolset);
+
+  if ((useToolset == "v140") || (useToolset == "v141") ||
+      (useToolset == "v142")) {
+    return "v14";
+  } else if (useToolset == "v120") {
+    return "v12";
+  } else if (useToolset == "v110") {
+    return "v11";
+  } else if (useToolset == "v100") {
+    return "v10";
+  } else {
+    return "";
+  }
+}
+
+std::string cmGlobalVisualStudio10Generator::CanonicalToolsetName(
+  std::string const& toolset) const
+{
+  std::size_t length = toolset.length();
+
+  if (cmHasLiteralSuffix(toolset, "_xp")) {
+    length -= 3;
+  }
+
+  return toolset.substr(0, length);
 }

Source/cmGlobalVisualStudio10Generator.h

@@ -5,8 +5,10 @@
 #include <memory>
 #include <set>
 
+#include <cm/optional>
+#include <cm/string_view>
+
 #include "cmGlobalVisualStudio8Generator.h"
-#include "cmVisualStudio10ToolsetOptions.h"
 
 /** \class cmGlobalVisualStudio10Generator
  * \brief Write a Unix makefiles.
@@ -170,8 +172,7 @@ protected:
 
   std::string const& GetMSBuildCommand();
 
-  cmIDEFlagTable const* LoadFlagTable(std::string const& optionsName,
-                                      std::string const& toolsetName,
+  cmIDEFlagTable const* LoadFlagTable(std::string const& toolSpecificName,
                                       std::string const& defaultName,
                                       std::string const& table) const;
 
@@ -224,7 +225,6 @@ private:
 
   std::string MSBuildCommand;
   bool MSBuildCommandInitialized;
-  cmVisualStudio10ToolsetOptions ToolsetOptions;
   std::set<std::string> AndroidExecutableWarnings;
   virtual std::string FindMSBuildCommand();
   std::string FindDevEnvCommand() override;
@@ -234,6 +234,17 @@ private:
 
   bool ParseGeneratorToolset(std::string const& ts, cmMakefile* mf);
 
+  std::string GetClFlagTableName() const;
+  std::string GetCSharpFlagTableName() const;
+  std::string GetRcFlagTableName() const;
+  std::string GetLibFlagTableName() const;
+  std::string GetLinkFlagTableName() const;
+  std::string GetMasmFlagTableName() const;
+  std::string CanonicalToolsetName(std::string const& toolset) const;
+
+  cm::optional<std::string> FindFlagTable(cm::string_view toolsetName,
+                                          cm::string_view table) const;
+
   std::string CustomVCTargetsPath;
   std::string VCTargetsPath;
   bool FindVCTargetsPath(cmMakefile* mf);

Source/cmVisualStudio10ToolsetOptions.cxx

@@ -1,143 +0,0 @@
-/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
-   file Copyright.txt or https://cmake.org/licensing for details.  */
-#include "cmVisualStudio10ToolsetOptions.h"
-
-#include "cmAlgorithms.h"
-#include "cmIDEFlagTable.h"
-#include "cmVisualStudioGeneratorOptions.h"
-
-std::string cmVisualStudio10ToolsetOptions::GetClFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if (toolset == "v142") {
-    return "v142";
-  } else if (toolset == "v141") {
-    return "v141";
-  } else if (useToolset == "v140") {
-    return "v140";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetCSharpFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if (useToolset == "v142") {
-    return "v142";
-  } else if (useToolset == "v141") {
-    return "v141";
-  } else if (useToolset == "v140") {
-    return "v140";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetRcFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if ((useToolset == "v140") || (useToolset == "v141") ||
-      (useToolset == "v142")) {
-    return "v14";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetLibFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if ((useToolset == "v140") || (useToolset == "v141") ||
-      (useToolset == "v142")) {
-    return "v14";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetLinkFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if (useToolset == "v142") {
-    return "v142";
-  } else if (useToolset == "v141") {
-    return "v141";
-  } else if (useToolset == "v140") {
-    return "v140";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetMasmFlagTableName(
-  std::string const& name, std::string const& toolset) const
-{
-  std::string const useToolset = this->GetToolsetName(name, toolset);
-
-  if ((useToolset == "v140") || (useToolset == "v141") ||
-      (useToolset == "v142")) {
-    return "v14";
-  } else if (useToolset == "v120") {
-    return "v12";
-  } else if (useToolset == "v110") {
-    return "v11";
-  } else if (useToolset == "v100") {
-    return "v10";
-  } else {
-    return "";
-  }
-}
-
-std::string cmVisualStudio10ToolsetOptions::GetToolsetName(
-  std::string const& name, std::string const& toolset) const
-{
-  static_cast<void>(name);
-  std::size_t length = toolset.length();
-
-  if (cmHasLiteralSuffix(toolset, "_xp")) {
-    length -= 3;
-  }
-
-  return toolset.substr(0, length);
-}

Source/cmVisualStudio10ToolsetOptions.h

@@ -1,31 +0,0 @@
-/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
-   file Copyright.txt or https://cmake.org/licensing for details.  */
-#pragma once
-
-#include "cmConfigure.h" // IWYU pragma: keep
-
-#include <string>
-
-/** \class cmVisualStudio10ToolsetOptions
- * \brief Retrieves toolset options for MSBuild.
- *
- * cmVisualStudio10ToolsetOptions manages toolsets within MSBuild
- */
-class cmVisualStudio10ToolsetOptions
-{
-public:
-  std::string GetClFlagTableName(std::string const& name,
-                                 std::string const& toolset) const;
-  std::string GetCSharpFlagTableName(std::string const& name,
-                                     std::string const& toolset) const;
-  std::string GetRcFlagTableName(std::string const& name,
-                                 std::string const& toolset) const;
-  std::string GetLibFlagTableName(std::string const& name,
-                                  std::string const& toolset) const;
-  std::string GetLinkFlagTableName(std::string const& name,
-                                   std::string const& toolset) const;
-  std::string GetMasmFlagTableName(std::string const& name,
-                                   std::string const& toolset) const;
-  std::string GetToolsetName(std::string const& name,
-                             std::string const& toolset) const;
-};