Merge topic 'cmSourceFile_refactor'

254b7260f4 cmSourceFile: Check if a file is GENERATED first in the full path computation
cd8a930d61 cmSourceFile: Refactor FindFullPath method
6d407ae439 Use cmSourceFile::GetIsGenerated
2ddf3f4467 cmSourceFile: Add IsGenerated method
b9d44fc350 cmSourceFile: Additional static property strings

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

13 files changed, 112 insertions, 84 deletions

diff --git a/Source/cmExtraCodeBlocksGenerator.cxx b/Source/cmExtraCodeBlocksGenerator.cxx
index 397508c..e408de3 100644
--- a/Source/cmExtraCodeBlocksGenerator.cxx
+++ b/Source/cmExtraCodeBlocksGenerator.cxx
@@ -362,7 +362,7 @@ void cmExtraCodeBlocksGenerator::CreateNewProjectFile(
             // don't add source files from UTILITY target which have the
             // GENERATED property set:
             if (gt->GetType() == cmStateEnums::UTILITY &&
-                s->GetPropertyAsBool("GENERATED")) {
+                s->GetIsGenerated()) {
               continue;
             }
 
diff --git a/Source/cmExtraKateGenerator.cxx b/Source/cmExtraKateGenerator.cxx
index e28a865..23ba6b7 100644
--- a/Source/cmExtraKateGenerator.cxx
+++ b/Source/cmExtraKateGenerator.cxx
@@ -256,7 +256,7 @@ std::string cmExtraKateGenerator::GenerateFilesString(
 
     const std::vector<cmSourceFile*>& sources = makefile->GetSourceFiles();
     for (cmSourceFile* sf : sources) {
-      if (sf->GetPropertyAsBool("GENERATED")) {
+      if (sf->GetIsGenerated()) {
         continue;
       }
 
diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx
index 4597d4f..45e8303 100644
--- a/Source/cmFileAPICodemodel.cxx
+++ b/Source/cmFileAPICodemodel.cxx
@@ -882,7 +882,7 @@ Json::Value Target::DumpSource(cmGeneratorTarget::SourceAndKind const& sk,
 
   std::string const path = sk.Source.Value->GetFullPath();
   source["path"] = RelativeIfUnder(this->TopSource, path);
-  if (sk.Source.Value->GetPropertyAsBool("GENERATED")) {
+  if (sk.Source.Value->GetIsGenerated()) {
     source["isGenerated"] = true;
   }
   this->AddBacktrace(source, sk.Source.Backtrace);
diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx
index f1b32ee..b428ae1 100644
--- a/Source/cmGlobalXCodeGenerator.cxx
+++ b/Source/cmGlobalXCodeGenerator.cxx
@@ -2460,7 +2460,7 @@ cmXCodeObject* cmGlobalXCodeGenerator::CreateUtilityTarget(
     this->AddXCodeProjBuildRule(gtgt, sources);
 
     for (auto sourceFile : sources) {
-      if (!sourceFile->GetPropertyAsBool("GENERATED")) {
+      if (!sourceFile->GetIsGenerated()) {
         this->CreateXCodeFileReference(sourceFile, gtgt);
       }
     }
diff --git a/Source/cmJsonObjects.cxx b/Source/cmJsonObjects.cxx
index 4b6b4d5..d723ced 100644
--- a/Source/cmJsonObjects.cxx
+++ b/Source/cmJsonObjects.cxx
@@ -323,7 +323,7 @@ static Json::Value DumpSourceFilesList(
       fileData.SetDefines(defines);
     }
 
-    fileData.IsGenerated = file->GetPropertyAsBool("GENERATED");
+    fileData.IsGenerated = file->GetIsGenerated();
     std::vector<std::string>& groupFileList = fileGroups[fileData];
     groupFileList.push_back(file->GetFullPath());
   }
diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx
index c29c657..5d76dc2 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -1000,7 +1000,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement(
   // (either attached to this source file or another one), assume that one of
   // the target dependencies, OBJECT_DEPENDS or header file custom commands
   // will rebuild the file.
-  if (source->GetPropertyAsBool("GENERATED") &&
+  if (source->GetIsGenerated() &&
       !source->GetPropertyAsBool("__CMAKE_GENERATED_BY_CMAKE") &&
       !source->GetCustomCommand() &&
       !this->GetGlobalGenerator()->HasCustomCommandOutput(sourceFileName)) {
diff --git a/Source/cmQTWrapCPPCommand.cxx b/Source/cmQTWrapCPPCommand.cxx
index d2133ed..6acc7ef 100644
--- a/Source/cmQTWrapCPPCommand.cxx
+++ b/Source/cmQTWrapCPPCommand.cxx
@@ -50,7 +50,7 @@ bool cmQTWrapCPPCommand::InitialPass(std::vector<std::string> const& args,
       if (cmSystemTools::FileIsFullPath(*j)) {
         hname = *j;
       } else {
-        if (curr && curr->GetPropertyAsBool("GENERATED")) {
+        if (curr && curr->GetIsGenerated()) {
           hname = this->Makefile->GetCurrentBinaryDirectory();
         } else {
           hname = this->Makefile->GetCurrentSourceDirectory();
diff --git a/Source/cmQTWrapUICommand.cxx b/Source/cmQTWrapUICommand.cxx
index 25dcd1a..43b1fb9 100644
--- a/Source/cmQTWrapUICommand.cxx
+++ b/Source/cmQTWrapUICommand.cxx
@@ -58,7 +58,7 @@ bool cmQTWrapUICommand::InitialPass(std::vector<std::string> const& args,
       if (cmSystemTools::FileIsFullPath(*j)) {
         uiName = *j;
       } else {
-        if (curr && curr->GetPropertyAsBool("GENERATED")) {
+        if (curr && curr->GetIsGenerated()) {
           uiName = this->Makefile->GetCurrentBinaryDirectory();
         } else {
           uiName = this->Makefile->GetCurrentSourceDirectory();
diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx
index caeed15..41d29e8 100644
--- a/Source/cmQtAutoGenInitializer.cxx
+++ b/Source/cmQtAutoGenInitializer.cxx
@@ -686,7 +686,7 @@ bool cmQtAutoGenInitializer::InitScanFiles()
           if ((this->Moc.Enabled && !sf->GetPropertyAsBool("SKIP_AUTOMOC")) ||
               (this->Uic.Enabled && !sf->GetPropertyAsBool("SKIP_AUTOUIC"))) {
             // Register source
-            const bool generated = sf->GetPropertyAsBool("GENERATED");
+            const bool generated = sf->GetIsGenerated();
             if (fileType == cmSystemTools::HEADER_FILE_FORMAT) {
               if (generated) {
                 this->AutogenTarget.HeadersGenerated.push_back(absPath);
@@ -712,7 +712,7 @@ bool cmQtAutoGenInitializer::InitScanFiles()
           qrc.QrcFile = cmSystemTools::GetRealPath(fPath);
           qrc.QrcName =
             cmSystemTools::GetFilenameWithoutLastExtension(qrc.QrcFile);
-          qrc.Generated = sf->GetPropertyAsBool("GENERATED");
+          qrc.Generated = sf->GetIsGenerated();
           // RCC options
           {
             std::string const opts = sf->GetSafeProperty("AUTORCC_OPTIONS");
diff --git a/Source/cmSourceFile.cxx b/Source/cmSourceFile.cxx
index efc6bb5..d05fb68 100644
--- a/Source/cmSourceFile.cxx
+++ b/Source/cmSourceFile.cxx
@@ -2,7 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmSourceFile.h"
 
-#include <sstream>
+#include <array>
+#include <utility>
 
 #include "cmCustomCommand.h"
 #include "cmGlobalGenerator.h"
@@ -17,8 +18,6 @@ cmSourceFile::cmSourceFile(cmMakefile* mf, const std::string& name,
                            cmSourceFileLocationKind kind)
   : Location(mf, name, kind)
 {
-  this->CustomCommand = nullptr;
-  this->FindFullPathFailed = false;
 }
 
 cmSourceFile::~cmSourceFile()
@@ -32,6 +31,8 @@ std::string const& cmSourceFile::GetExtension() const
 }
 
 const std::string cmSourceFile::propLANGUAGE = "LANGUAGE";
+const std::string cmSourceFile::propLOCATION = "LOCATION";
+const std::string cmSourceFile::propGENERATED = "GENERATED";
 
 void cmSourceFile::SetObjectLibrary(std::string const& objlib)
 {
@@ -112,92 +113,88 @@ std::string const& cmSourceFile::GetFullPath() const
 
 bool cmSourceFile::FindFullPath(std::string* error)
 {
-  // If this method has already failed once do not try again.
-  if (this->FindFullPathFailed) {
-    return false;
-  }
-
-  // If the file is generated compute the location without checking on
-  // disk.
-  if (this->GetPropertyAsBool("GENERATED")) {
+  // If the file is generated compute the location without checking on disk.
+  if (this->GetIsGenerated()) {
     // The file is either already a full path or is relative to the
     // build directory for the target.
     this->Location.DirectoryUseBinary();
-    this->FullPath = this->Location.GetDirectory();
-    this->FullPath += "/";
-    this->FullPath += this->Location.GetName();
+    this->FullPath = this->Location.GetFullPath();
     return true;
   }
 
-  // The file is not generated.  It must exist on disk.
-  cmMakefile const* mf = this->Location.GetMakefile();
-  const char* tryDirs[3] = { nullptr, nullptr, nullptr };
-  if (this->Location.DirectoryIsAmbiguous()) {
-    tryDirs[0] = mf->GetCurrentSourceDirectory().c_str();
-    tryDirs[1] = mf->GetCurrentBinaryDirectory().c_str();
-  } else {
-    tryDirs[0] = "";
+  // If this method has already failed once do not try again.
+  if (this->FindFullPathFailed) {
+    return false;
   }
 
-  cmake const* const cmakeInst = mf->GetCMakeInstance();
-  std::vector<std::string> const& srcExts = cmakeInst->GetSourceExtensions();
-  std::vector<std::string> const& hdrExts = cmakeInst->GetHeaderExtensions();
-  for (const char* const* di = tryDirs; *di; ++di) {
-    std::string tryPath = this->Location.GetDirectory();
-    if (!tryPath.empty()) {
-      tryPath += "/";
-    }
-    tryPath += this->Location.GetName();
-    tryPath = cmSystemTools::CollapseFullPath(tryPath, *di);
-    if (this->TryFullPath(tryPath, "")) {
+  // The file is not generated.  It must exist on disk.
+  cmMakefile const* makefile = this->Location.GetMakefile();
+  // Location path
+  std::string const lPath = this->Location.GetFullPath();
+  // List of extension lists
+  std::array<std::vector<std::string> const*, 2> const extsLists = {
+    { &makefile->GetCMakeInstance()->GetSourceExtensions(),
+      &makefile->GetCMakeInstance()->GetHeaderExtensions() }
+  };
+
+  // Tries to find the file in a given directory
+  auto findInDir = [this, &extsLists, &lPath](std::string const& dir) -> bool {
+    // Compute full path
+    std::string const fullPath = cmSystemTools::CollapseFullPath(lPath, dir);
+    // Try full path
+    if (cmSystemTools::FileExists(fullPath)) {
+      this->FullPath = fullPath;
       return true;
     }
-    for (std::string const& ext : srcExts) {
-      if (this->TryFullPath(tryPath, ext)) {
-        return true;
+    // Try full path with extension
+    for (auto exts : extsLists) {
+      for (std::string const& ext : *exts) {
+        if (!ext.empty()) {
+          std::string extPath = fullPath;
+          extPath += '.';
+          extPath += ext;
+          if (cmSystemTools::FileExists(extPath)) {
+            this->FullPath = extPath;
+            return true;
+          }
+        }
       }
     }
-    for (std::string const& ext : hdrExts) {
-      if (this->TryFullPath(tryPath, ext)) {
-        return true;
-      }
+    // File not found
+    return false;
+  };
+
+  // Try to find the file in various directories
+  if (this->Location.DirectoryIsAmbiguous()) {
+    if (findInDir(makefile->GetCurrentSourceDirectory()) ||
+        findInDir(makefile->GetCurrentBinaryDirectory())) {
+      return true;
+    }
+  } else {
+    if (findInDir({})) {
+      return true;
     }
   }
 
-  std::ostringstream e;
-  std::string missing = this->Location.GetDirectory();
-  if (!missing.empty()) {
-    missing += "/";
-  }
-  missing += this->Location.GetName();
-  e << "Cannot find source file:\n  " << missing << "\nTried extensions";
-  for (std::string const& srcExt : srcExts) {
-    e << " ." << srcExt;
-  }
-  for (std::string const& ext : hdrExts) {
-    e << " ." << ext;
+  // Compose error
+  std::string err;
+  err += "Cannot find source file:\n  ";
+  err += lPath;
+  err += "\nTried extensions";
+  for (auto exts : extsLists) {
+    for (std::string const& ext : *exts) {
+      err += " .";
+      err += ext;
+    }
   }
-  if (error) {
-    *error = e.str();
+  if (error != nullptr) {
+    *error = std::move(err);
   } else {
-    this->Location.GetMakefile()->IssueMessage(MessageType::FATAL_ERROR,
-                                               e.str());
+    makefile->IssueMessage(MessageType::FATAL_ERROR, err);
   }
   this->FindFullPathFailed = true;
-  return false;
-}
 
-bool cmSourceFile::TryFullPath(const std::string& path, const std::string& ext)
-{
-  std::string tryPath = path;
-  if (!ext.empty()) {
-    tryPath += ".";
-    tryPath += ext;
-  }
-  if (cmSystemTools::FileExists(tryPath)) {
-    this->FullPath = tryPath;
-    return true;
-  }
+  // File not found
   return false;
 }
 
@@ -242,12 +239,22 @@ bool cmSourceFile::Matches(cmSourceFileLocation const& loc)
 void cmSourceFile::SetProperty(const std::string& prop, const char* value)
 {
   this->Properties.SetProperty(prop, value);
+
+  // Update IsGenerated flag
+  if (prop == propGENERATED) {
+    this->IsGenerated = cmSystemTools::IsOn(value);
+  }
 }
 
 void cmSourceFile::AppendProperty(const std::string& prop, const char* value,
                                   bool asString)
 {
   this->Properties.AppendProperty(prop, value, asString);
+
+  // Update IsGenerated flag
+  if (prop == propGENERATED) {
+    this->IsGenerated = this->GetPropertyAsBool(propGENERATED);
+  }
 }
 
 const char* cmSourceFile::GetPropertyForUser(const std::string& prop)
@@ -266,7 +273,7 @@ const char* cmSourceFile::GetPropertyForUser(const std::string& prop)
   // cmSourceFileLocation class to commit to a particular full path to
   // the source file as late as possible.  If the users requests the
   // LOCATION property we must commit now.
-  if (prop == "LOCATION") {
+  if (prop == propLOCATION) {
     // Commit to a location.
     this->GetFullPath();
   }
@@ -278,7 +285,7 @@ const char* cmSourceFile::GetPropertyForUser(const std::string& prop)
 const char* cmSourceFile::GetProperty(const std::string& prop) const
 {
   // Check for computed properties.
-  if (prop == "LOCATION") {
+  if (prop == propLOCATION) {
     if (this->FullPath.empty()) {
       return nullptr;
     }
diff --git a/Source/cmSourceFile.h b/Source/cmSourceFile.h
index ab0f229..a82a58a 100644
--- a/Source/cmSourceFile.h
+++ b/Source/cmSourceFile.h
@@ -55,6 +55,10 @@ public:
       command like get_property or get_source_file_property.  */
   const char* GetPropertyForUser(const std::string& prop);
 
+  ///! Checks is the GENERATED property is set and true
+  /// @return Equivalent to GetPropertyAsBool("GENERATED")
+  bool GetIsGenerated() const { return this->IsGenerated; }
+
   /**
    * The full path to the file.  The non-const version of this method
    * may attempt to locate the file on disk and finalize its location.
@@ -106,20 +110,22 @@ public:
 private:
   cmSourceFileLocation Location;
   cmPropertyMap Properties;
-  cmCustomCommand* CustomCommand;
+  cmCustomCommand* CustomCommand = nullptr;
   std::string Extension;
   std::string Language;
   std::string FullPath;
   std::string ObjectLibrary;
   std::vector<std::string> Depends;
-  bool FindFullPathFailed;
+  bool FindFullPathFailed = false;
+  bool IsGenerated = false;
 
   bool FindFullPath(std::string* error);
-  bool TryFullPath(const std::string& path, const std::string& ext);
   void CheckExtension();
   void CheckLanguage(std::string const& ext);
 
   static const std::string propLANGUAGE;
+  static const std::string propLOCATION;
+  static const std::string propGENERATED;
 };
 
 // TODO: Factor out into platform information modules.
diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx
index 13d2d7e..acacba2 100644
--- a/Source/cmSourceFileLocation.cxx
+++ b/Source/cmSourceFileLocation.cxx
@@ -42,6 +42,16 @@ cmSourceFileLocation::cmSourceFileLocation(cmMakefile const* mf,
   }
 }
 
+std::string cmSourceFileLocation::GetFullPath() const
+{
+  std::string path = this->GetDirectory();
+  if (!path.empty()) {
+    path += '/';
+  }
+  path += this->GetName();
+  return path;
+}
+
 void cmSourceFileLocation::Update(cmSourceFileLocation const& loc)
 {
   if (this->AmbiguousDirectory && !loc.AmbiguousDirectory) {
diff --git a/Source/cmSourceFileLocation.h b/Source/cmSourceFileLocation.h
index cb35703..87040b8 100644
--- a/Source/cmSourceFileLocation.h
+++ b/Source/cmSourceFileLocation.h
@@ -80,6 +80,11 @@ public:
   const std::string& GetName() const { return this->Name; }
 
   /**
+   * Get the full file path composed of GetDirectory() and GetName().
+   */
+  std::string GetFullPath() const;
+
+  /**
    * Get the cmMakefile instance for which the source file was created.
    */
   cmMakefile const* GetMakefile() const { return this->Makefile; }