Merge topic 'perf-from-callgrind'

901c4a1e cmExpandedCommandArgument: add an overload for const char*
88ed556d cmGeneratorTarget: make keyword strings const
14a13d30 cmGeneratorExpressionLexer: only tokenize strings with a '$'
f2b8d67f cmTarget: use static strings for special property names
6dfd0f92 cmGeneratorExpressionNode: avoid some strlen in $<TARGET_PROPERTY>
f9235fd4 cmAddCustomCommandCommand: use std::string const& for FileIsFullPath
c0e7a137 cmAddCustomCommandCommand: store keywords in strings

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

Source/cmAddCustomCommandCommand.cxx

@@ -3,6 +3,7 @@
 #include "cmAddCustomCommandCommand.h"
 
 #include <sstream>
+#include <unordered_set>
 #include <utility>
 
 #include "cmCustomCommand.h"
@@ -69,57 +70,106 @@ bool cmAddCustomCommandCommand::InitialPass(
 
   tdoing doing = doing_nothing;
 
-  for (std::string const& copy : args) {
-    if (copy == "SOURCE") {
-      doing = doing_source;
-    } else if (copy == "COMMAND") {
-      doing = doing_command;
+#define MAKE_STATIC_KEYWORD(KEYWORD)                                          \
+  static const std::string key##KEYWORD = #KEYWORD
+  MAKE_STATIC_KEYWORD(APPEND);
+  MAKE_STATIC_KEYWORD(ARGS);
+  MAKE_STATIC_KEYWORD(BYPRODUCTS);
+  MAKE_STATIC_KEYWORD(COMMAND);
+  MAKE_STATIC_KEYWORD(COMMAND_EXPAND_LISTS);
+  MAKE_STATIC_KEYWORD(COMMENT);
+  MAKE_STATIC_KEYWORD(DEPENDS);
+  MAKE_STATIC_KEYWORD(DEPFILE);
+  MAKE_STATIC_KEYWORD(IMPLICIT_DEPENDS);
+  MAKE_STATIC_KEYWORD(MAIN_DEPENDENCY);
+  MAKE_STATIC_KEYWORD(OUTPUT);
+  MAKE_STATIC_KEYWORD(OUTPUTS);
+  MAKE_STATIC_KEYWORD(POST_BUILD);
+  MAKE_STATIC_KEYWORD(PRE_BUILD);
+  MAKE_STATIC_KEYWORD(PRE_LINK);
+  MAKE_STATIC_KEYWORD(SOURCE);
+  MAKE_STATIC_KEYWORD(TARGET);
+  MAKE_STATIC_KEYWORD(USES_TERMINAL);
+  MAKE_STATIC_KEYWORD(VERBATIM);
+  MAKE_STATIC_KEYWORD(WORKING_DIRECTORY);
+#undef MAKE_STATIC_KEYWORD
+  static std::unordered_set<std::string> keywords;
+  if (keywords.empty()) {
+    keywords.insert(keyAPPEND);
+    keywords.insert(keyARGS);
+    keywords.insert(keyBYPRODUCTS);
+    keywords.insert(keyCOMMAND);
+    keywords.insert(keyCOMMAND_EXPAND_LISTS);
+    keywords.insert(keyCOMMENT);
+    keywords.insert(keyDEPENDS);
+    keywords.insert(keyDEPFILE);
+    keywords.insert(keyIMPLICIT_DEPENDS);
+    keywords.insert(keyMAIN_DEPENDENCY);
+    keywords.insert(keyOUTPUT);
+    keywords.insert(keyOUTPUTS);
+    keywords.insert(keyPOST_BUILD);
+    keywords.insert(keyPRE_BUILD);
+    keywords.insert(keyPRE_LINK);
+    keywords.insert(keySOURCE);
+    keywords.insert(keyTARGET);
+    keywords.insert(keyUSES_TERMINAL);
+    keywords.insert(keyVERBATIM);
+    keywords.insert(keyWORKING_DIRECTORY);
+  }
 
-      // Save the current command before starting the next command.
-      if (!currentLine.empty()) {
-        commandLines.push_back(currentLine);
-        currentLine.clear();
-      }
-    } else if (copy == "PRE_BUILD") {
-      cctype = cmTarget::PRE_BUILD;
-    } else if (copy == "PRE_LINK") {
-      cctype = cmTarget::PRE_LINK;
-    } else if (copy == "POST_BUILD") {
-      cctype = cmTarget::POST_BUILD;
-    } else if (copy == "VERBATIM") {
-      verbatim = true;
-    } else if (copy == "APPEND") {
-      append = true;
-    } else if (copy == "USES_TERMINAL") {
-      uses_terminal = true;
-    } else if (copy == "COMMAND_EXPAND_LISTS") {
-      command_expand_lists = true;
-    } else if (copy == "TARGET") {
-      doing = doing_target;
-    } else if (copy == "ARGS") {
-      // Ignore this old keyword.
-    } else if (copy == "DEPENDS") {
-      doing = doing_depends;
-    } else if (copy == "OUTPUTS") {
-      doing = doing_outputs;
-    } else if (copy == "OUTPUT") {
-      doing = doing_output;
-    } else if (copy == "BYPRODUCTS") {
-      doing = doing_byproducts;
-    } else if (copy == "WORKING_DIRECTORY") {
-      doing = doing_working_directory;
-    } else if (copy == "MAIN_DEPENDENCY") {
-      doing = doing_main_dependency;
-    } else if (copy == "IMPLICIT_DEPENDS") {
-      doing = doing_implicit_depends_lang;
-    } else if (copy == "COMMENT") {
-      doing = doing_comment;
-    } else if (copy == "DEPFILE") {
-      doing = doing_depfile;
-      if (this->Makefile->GetGlobalGenerator()->GetName() != "Ninja") {
-        this->SetError("Option DEPFILE not supported by " +
-                       this->Makefile->GetGlobalGenerator()->GetName());
-        return false;
+  for (std::string const& copy : args) {
+    if (keywords.count(copy)) {
+      if (copy == keySOURCE) {
+        doing = doing_source;
+      } else if (copy == keyCOMMAND) {
+        doing = doing_command;
+
+        // Save the current command before starting the next command.
+        if (!currentLine.empty()) {
+          commandLines.push_back(currentLine);
+          currentLine.clear();
+        }
+      } else if (copy == keyPRE_BUILD) {
+        cctype = cmTarget::PRE_BUILD;
+      } else if (copy == keyPRE_LINK) {
+        cctype = cmTarget::PRE_LINK;
+      } else if (copy == keyPOST_BUILD) {
+        cctype = cmTarget::POST_BUILD;
+      } else if (copy == keyVERBATIM) {
+        verbatim = true;
+      } else if (copy == keyAPPEND) {
+        append = true;
+      } else if (copy == keyUSES_TERMINAL) {
+        uses_terminal = true;
+      } else if (copy == keyCOMMAND_EXPAND_LISTS) {
+        command_expand_lists = true;
+      } else if (copy == keyTARGET) {
+        doing = doing_target;
+      } else if (copy == keyARGS) {
+        // Ignore this old keyword.
+      } else if (copy == keyDEPENDS) {
+        doing = doing_depends;
+      } else if (copy == keyOUTPUTS) {
+        doing = doing_outputs;
+      } else if (copy == keyOUTPUT) {
+        doing = doing_output;
+      } else if (copy == keyBYPRODUCTS) {
+        doing = doing_byproducts;
+      } else if (copy == keyWORKING_DIRECTORY) {
+        doing = doing_working_directory;
+      } else if (copy == keyMAIN_DEPENDENCY) {
+        doing = doing_main_dependency;
+      } else if (copy == keyIMPLICIT_DEPENDS) {
+        doing = doing_implicit_depends_lang;
+      } else if (copy == keyCOMMENT) {
+        doing = doing_comment;
+      } else if (copy == keyDEPFILE) {
+        doing = doing_depfile;
+        if (this->Makefile->GetGlobalGenerator()->GetName() != "Ninja") {
+          this->SetError("Option DEPFILE not supported by " +
+                         this->Makefile->GetGlobalGenerator()->GetName());
+          return false;
+        }
       }
     } else {
       std::string filename;
@@ -127,7 +177,7 @@ bool cmAddCustomCommandCommand::InitialPass(
         case doing_output:
         case doing_outputs:
         case doing_byproducts:
-          if (!cmSystemTools::FileIsFullPath(copy.c_str())) {
+          if (!cmSystemTools::FileIsFullPath(copy)) {
             // This is an output to be generated, so it should be
             // under the build tree.  CMake 2.4 placed this under the
             // source tree.  However the only case that this change

Source/cmExpandedCommandArgument.cxx

@@ -24,6 +24,11 @@ bool cmExpandedCommandArgument::WasQuoted() const
   return this->Quoted;
 }
 
+bool cmExpandedCommandArgument::operator==(const char* value) const
+{
+  return this->Value == value;
+}
+
 bool cmExpandedCommandArgument::operator==(std::string const& value) const
 {
   return this->Value == value;

Source/cmExpandedCommandArgument.h

@@ -24,6 +24,7 @@ public:
 
   bool WasQuoted() const;
 
+  bool operator==(const char* value) const;
   bool operator==(std::string const& value) const;
 
   bool empty() const;

Source/cmGeneratorExpressionLexer.cxx

@@ -21,6 +21,12 @@ std::vector<cmGeneratorExpressionToken> cmGeneratorExpressionLexer::Tokenize(
 {
   std::vector<cmGeneratorExpressionToken> result;
 
+  if (input.find('$') == std::string::npos) {
+    result.push_back(cmGeneratorExpressionToken(
+      cmGeneratorExpressionToken::Text, input.c_str(), input.size()));
+    return result;
+  }
+
   const char* c = input.c_str();
   const char* upto = c;
 

Source/cmGeneratorExpressionNode.cxx

@@ -947,7 +947,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode
                       "Target name not supported.");
         return std::string();
       }
-      if (propertyName == "ALIASED_TARGET") {
+      static const std::string propALIASED_TARGET = "ALIASED_TARGET";
+      if (propertyName == propALIASED_TARGET) {
         if (context->LG->GetMakefile()->IsAlias(targetName)) {
           if (cmGeneratorTarget* tgt =
                 context->LG->FindGeneratorTargetToUse(targetName)) {

Source/cmGeneratorTarget.cxx

@@ -3616,13 +3616,13 @@ void cmGeneratorTarget::CheckPropertyCompatibility(
   const cmComputeLinkInformation::ItemVector& deps = info->GetItems();
 
   std::set<std::string> emittedBools;
-  static std::string strBool = "COMPATIBLE_INTERFACE_BOOL";
+  static const std::string strBool = "COMPATIBLE_INTERFACE_BOOL";
   std::set<std::string> emittedStrings;
-  static std::string strString = "COMPATIBLE_INTERFACE_STRING";
+  static const std::string strString = "COMPATIBLE_INTERFACE_STRING";
   std::set<std::string> emittedMinNumbers;
-  static std::string strNumMin = "COMPATIBLE_INTERFACE_NUMBER_MIN";
+  static const std::string strNumMin = "COMPATIBLE_INTERFACE_NUMBER_MIN";
   std::set<std::string> emittedMaxNumbers;
-  static std::string strNumMax = "COMPATIBLE_INTERFACE_NUMBER_MAX";
+  static const std::string strNumMax = "COMPATIBLE_INTERFACE_NUMBER_MAX";
 
   for (auto const& dep : deps) {
     if (!dep.Target) {

Source/cmTarget.cxx

@@ -861,39 +861,53 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
         this->Makefile->GetBacktrace())) {
     return;
   }
-  if (prop == "MANUALLY_ADDED_DEPENDENCIES") {
+#define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP
+  MAKE_STATIC_PROP(COMPILE_DEFINITIONS);
+  MAKE_STATIC_PROP(COMPILE_FEATURES);
+  MAKE_STATIC_PROP(COMPILE_OPTIONS);
+  MAKE_STATIC_PROP(CUDA_PTX_COMPILATION);
+  MAKE_STATIC_PROP(EXPORT_NAME);
+  MAKE_STATIC_PROP(IMPORTED_GLOBAL);
+  MAKE_STATIC_PROP(INCLUDE_DIRECTORIES);
+  MAKE_STATIC_PROP(LINK_LIBRARIES);
+  MAKE_STATIC_PROP(MANUALLY_ADDED_DEPENDENCIES);
+  MAKE_STATIC_PROP(NAME);
+  MAKE_STATIC_PROP(SOURCES);
+  MAKE_STATIC_PROP(TYPE);
+#undef MAKE_STATIC_PROP
+  if (prop == propMANUALLY_ADDED_DEPENDENCIES) {
     std::ostringstream e;
     e << "MANUALLY_ADDED_DEPENDENCIES property is read-only\n";
     this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     return;
   }
-  if (prop == "NAME") {
+  if (prop == propNAME) {
     std::ostringstream e;
     e << "NAME property is read-only\n";
     this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     return;
   }
-  if (prop == "TYPE") {
+  if (prop == propTYPE) {
     std::ostringstream e;
     e << "TYPE property is read-only\n";
     this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     return;
   }
-  if (prop == "EXPORT_NAME" && this->IsImported()) {
+  if (prop == propEXPORT_NAME && this->IsImported()) {
     std::ostringstream e;
     e << "EXPORT_NAME property can't be set on imported targets (\""
       << this->Name << "\")\n";
     this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     return;
   }
-  if (prop == "SOURCES" && this->IsImported()) {
+  if (prop == propSOURCES && this->IsImported()) {
     std::ostringstream e;
     e << "SOURCES property can't be set on imported targets (\"" << this->Name
       << "\")\n";
     this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str());
     return;
   }
-  if (prop == "IMPORTED_GLOBAL" && !this->IsImported()) {
+  if (prop == propIMPORTED_GLOBAL && !this->IsImported()) {
     std::ostringstream e;
     e << "IMPORTED_GLOBAL property can't be set on non-imported targets (\""
       << this->Name << "\")\n";
@@ -901,7 +915,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
     return;
   }
 
-  if (prop == "INCLUDE_DIRECTORIES") {
+  if (prop == propINCLUDE_DIRECTORIES) {
     this->Internal->IncludeDirectoriesEntries.clear();
     this->Internal->IncludeDirectoriesBacktraces.clear();
     if (value) {
@@ -909,7 +923,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       cmListFileBacktrace lfbt = this->Makefile->GetBacktrace();
       this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt);
     }
-  } else if (prop == "COMPILE_OPTIONS") {
+  } else if (prop == propCOMPILE_OPTIONS) {
     this->Internal->CompileOptionsEntries.clear();
     this->Internal->CompileOptionsBacktraces.clear();
     if (value) {
@@ -917,7 +931,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       cmListFileBacktrace lfbt = this->Makefile->GetBacktrace();
       this->Internal->CompileOptionsBacktraces.push_back(lfbt);
     }
-  } else if (prop == "COMPILE_FEATURES") {
+  } else if (prop == propCOMPILE_FEATURES) {
     this->Internal->CompileFeaturesEntries.clear();
     this->Internal->CompileFeaturesBacktraces.clear();
     if (value) {
@@ -925,7 +939,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       cmListFileBacktrace lfbt = this->Makefile->GetBacktrace();
       this->Internal->CompileFeaturesBacktraces.push_back(lfbt);
     }
-  } else if (prop == "COMPILE_DEFINITIONS") {
+  } else if (prop == propCOMPILE_DEFINITIONS) {
     this->Internal->CompileDefinitionsEntries.clear();
     this->Internal->CompileDefinitionsBacktraces.clear();
     if (value) {
@@ -933,7 +947,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       cmListFileBacktrace lfbt = this->Makefile->GetBacktrace();
       this->Internal->CompileDefinitionsBacktraces.push_back(lfbt);
     }
-  } else if (prop == "LINK_LIBRARIES") {
+  } else if (prop == propLINK_LIBRARIES) {
     this->Internal->LinkImplementationPropertyEntries.clear();
     this->Internal->LinkImplementationPropertyBacktraces.clear();
     if (value) {
@@ -941,7 +955,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       this->Internal->LinkImplementationPropertyEntries.push_back(value);
       this->Internal->LinkImplementationPropertyBacktraces.push_back(lfbt);
     }
-  } else if (prop == "SOURCES") {
+  } else if (prop == propSOURCES) {
     this->Internal->SourceEntries.clear();
     this->Internal->SourceBacktraces.clear();
     if (value) {
@@ -949,7 +963,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
       this->Internal->SourceEntries.push_back(value);
       this->Internal->SourceBacktraces.push_back(lfbt);
     }
-  } else if (prop == "IMPORTED_GLOBAL") {
+  } else if (prop == propIMPORTED_GLOBAL) {
     if (!cmSystemTools::IsOn(value)) {
       std::ostringstream e;
       e << "IMPORTED_GLOBAL property can't be set to FALSE on targets (\""
@@ -965,7 +979,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value)
   } else if (cmHasLiteralPrefix(prop, "IMPORTED_LIBNAME") &&
              !this->CheckImportedLibName(prop, value ? value : "")) {
     /* error was reported by check method */
-  } else if (prop == "CUDA_PTX_COMPILATION" &&
+  } else if (prop == propCUDA_PTX_COMPILATION &&
              this->GetType() != cmStateEnums::OBJECT_LIBRARY) {
     std::ostringstream e;
     e << "CUDA_PTX_COMPILATION property can only be applied to OBJECT "