Merge topic 'code-improvements'

0c47ed6430 cmMakefile: Convert private helpers to file static functions
e13fa223fc cmMakefile: Improve ExpandVariablesInString return type
b542e0c74f cmCPluginAPI: Remove a few unnecessary c_str() calls

Acked-by: Kitware Robot <[email protected]>
Rejected-by: Marc Chevrier <[email protected]>
Merge-request: !2018

Source/cmCPluginAPI.cxx

@@ -405,7 +405,8 @@ char CCONV* cmExpandVariablesInString(void* arg, const char* source,
 {
   cmMakefile* mf = static_cast<cmMakefile*>(arg);
   std::string barf = source;
-  std::string result = mf->ExpandVariablesInString(barf, escapeQuotes, atOnly);
+  std::string const& result =
+    mf->ExpandVariablesInString(barf, escapeQuotes, atOnly);
   return strdup(result.c_str());
 }
 
@@ -664,7 +665,7 @@ void CCONV cmSourceFileSetName(void* arg, const char* name, const char* dir,
   // First try and see whether the listed file can be found
   // as is without extensions added on.
   std::string hname = pathname;
-  if (cmSystemTools::FileExists(hname.c_str())) {
+  if (cmSystemTools::FileExists(hname)) {
     sf->SourceName = cmSystemTools::GetFilenamePath(name);
     if (!sf->SourceName.empty()) {
       sf->SourceName += "/";
@@ -691,7 +692,7 @@ void CCONV cmSourceFileSetName(void* arg, const char* name, const char* dir,
     hname = pathname;
     hname += ".";
     hname += *ext;
-    if (cmSystemTools::FileExists(hname.c_str())) {
+    if (cmSystemTools::FileExists(hname)) {
       sf->SourceExtension = *ext;
       sf->FullPath = hname;
       return;
@@ -704,7 +705,7 @@ void CCONV cmSourceFileSetName(void* arg, const char* name, const char* dir,
     hname = pathname;
     hname += ".";
     hname += *ext;
-    if (cmSystemTools::FileExists(hname.c_str())) {
+    if (cmSystemTools::FileExists(hname)) {
       sf->SourceExtension = *ext;
       sf->FullPath = hname;
       return;

Source/cmMakefile.cxx

@@ -1130,57 +1130,38 @@ cmTarget* cmMakefile::AddUtilityCommand(
   return target;
 }
 
-void cmMakefile::AddDefineFlag(std::string const& flag)
-{
-  if (flag.empty()) {
-    return;
-  }
-
-  // Update the string used for the old DEFINITIONS property.
-  this->AddDefineFlag(flag, this->DefineFlagsOrig);
-
-  // If this is really a definition, update COMPILE_DEFINITIONS.
-  if (this->ParseDefineFlag(flag, false)) {
-    return;
-  }
-
-  // Add this flag that does not look like a definition.
-  this->AddDefineFlag(flag, this->DefineFlags);
-}
-
-void cmMakefile::AddDefineFlag(std::string const& flag, std::string& dflags)
+static void s_AddDefineFlag(std::string const& flag, std::string& dflags)
 {
   // remove any \n\r
   std::string::size_type initSize = dflags.size();
-  dflags += std::string(" ") + flag;
+  dflags += ' ';
+  dflags += flag;
   std::string::iterator flagStart = dflags.begin() + initSize + 1;
   std::replace(flagStart, dflags.end(), '\n', ' ');
   std::replace(flagStart, dflags.end(), '\r', ' ');
 }
 
-void cmMakefile::RemoveDefineFlag(std::string const& flag)
+void cmMakefile::AddDefineFlag(std::string const& flag)
 {
-  // Check the length of the flag to remove.
   if (flag.empty()) {
     return;
   }
-  std::string::size_type const len = flag.length();
+
   // Update the string used for the old DEFINITIONS property.
-  this->RemoveDefineFlag(flag, len, this->DefineFlagsOrig);
+  s_AddDefineFlag(flag, this->DefineFlagsOrig);
 
   // If this is really a definition, update COMPILE_DEFINITIONS.
-  if (this->ParseDefineFlag(flag, true)) {
+  if (this->ParseDefineFlag(flag, false)) {
     return;
   }
 
-  // Remove this flag that does not look like a definition.
-  this->RemoveDefineFlag(flag, len, this->DefineFlags);
+  // Add this flag that does not look like a definition.
+  s_AddDefineFlag(flag, this->DefineFlags);
 }
 
-void cmMakefile::RemoveDefineFlag(std::string const& flag,
-                                  std::string::size_type len,
-                                  std::string& dflags)
+static void s_RemoveDefineFlag(std::string const& flag, std::string& dflags)
 {
+  std::string::size_type const len = flag.length();
   // Remove all instances of the flag that are surrounded by
   // whitespace or the beginning/end of the string.
   for (std::string::size_type lpos = dflags.find(flag, 0);
@@ -1195,6 +1176,25 @@ void cmMakefile::RemoveDefineFlag(std::string const& flag,
   }
 }
 
+void cmMakefile::RemoveDefineFlag(std::string const& flag)
+{
+  // Check the length of the flag to remove.
+  if (flag.empty()) {
+    return;
+  }
+
+  // Update the string used for the old DEFINITIONS property.
+  s_RemoveDefineFlag(flag, this->DefineFlagsOrig);
+
+  // If this is really a definition, update COMPILE_DEFINITIONS.
+  if (this->ParseDefineFlag(flag, true)) {
+    return;
+  }
+
+  // Remove this flag that does not look like a definition.
+  s_RemoveDefineFlag(flag, this->DefineFlags);
+}
+
 void cmMakefile::AddCompileDefinition(std::string const& option)
 {
   this->AppendProperty("COMPILE_DEFINITIONS", option.c_str());
@@ -2412,12 +2412,13 @@ std::vector<std::string> cmMakefile::GetDefinitions() const
   return res;
 }
 
-const char* cmMakefile::ExpandVariablesInString(std::string& source) const
+const std::string& cmMakefile::ExpandVariablesInString(
+  std::string& source) const
 {
   return this->ExpandVariablesInString(source, false, false);
 }
 
-const char* cmMakefile::ExpandVariablesInString(
+const std::string& cmMakefile::ExpandVariablesInString(
   std::string& source, bool escapeQuotes, bool noEscapes, bool atOnly,
   const char* filename, long line, bool removeEmpty, bool replaceAt) const
 {
@@ -2433,7 +2434,7 @@ const char* cmMakefile::ExpandVariablesInString(
     this->IssueMessage(cmake::INTERNAL_ERROR,
                        "ExpandVariablesInString @ONLY called "
                        "on something with escapes.");
-    return source.c_str();
+    return source;
   }
 
   // Variables used in the WARN case.
@@ -2515,7 +2516,7 @@ const char* cmMakefile::ExpandVariablesInString(
     this->IssueMessage(cmake::AUTHOR_WARNING, msg);
   }
 
-  return source.c_str();
+  return source;
 }
 
 cmake::MessageType cmMakefile::ExpandVariablesInStringOld(

Source/cmMakefile.h

@@ -565,12 +565,11 @@ public:
    * entry in the this->Definitions map.  Also \@var\@ is
    * expanded to match autoconf style expansions.
    */
-  const char* ExpandVariablesInString(std::string& source) const;
-  const char* ExpandVariablesInString(std::string& source, bool escapeQuotes,
-                                      bool noEscapes, bool atOnly = false,
-                                      const char* filename = nullptr,
-                                      long line = -1, bool removeEmpty = false,
-                                      bool replaceAt = false) const;
+  const std::string& ExpandVariablesInString(std::string& source) const;
+  const std::string& ExpandVariablesInString(
+    std::string& source, bool escapeQuotes, bool noEscapes,
+    bool atOnly = false, const char* filename = nullptr, long line = -1,
+    bool removeEmpty = false, bool replaceAt = false) const;
 
   /**
    * Remove any remaining variables in the string. Anything with ${var} or
@@ -886,9 +885,6 @@ protected:
   std::string DefineFlags;
 
   // Track the value of the computed DEFINITIONS property.
-  void AddDefineFlag(std::string const& flag, std::string&);
-  void RemoveDefineFlag(std::string const& flag, std::string::size_type,
-                        std::string&);
   std::string DefineFlagsOrig;
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)