Merge pull request #9005 from diffblue/string_utils_string_view

use `std::string_view` in string_utils

Author: tautschnig

src/util/string_utils.cpp

@@ -18,26 +18,27 @@ Author: Daniel Poetzl
 /// in the middle of the string is left unchanged
 /// \param s: the string to strip
 /// \return The stripped string
-std::string strip_string(const std::string &s)
+std::string strip_string(std::string_view s)
 {
   auto pred=[](char c){ return std::isspace(c); };
 
-  std::string::const_iterator left
-    =std::find_if_not(s.begin(), s.end(), pred);
+  std::string_view::const_iterator left =
+    std::find_if_not(s.begin(), s.end(), pred);
   if(left==s.end())
     return "";
 
-  std::string::size_type i=std::distance(s.begin(), left);
+  std::size_t i = std::distance(s.begin(), left);
 
-  std::string::const_reverse_iterator right
-    =std::find_if_not(s.rbegin(), s.rend(), pred);
-  std::string::size_type j=std::distance(right, s.rend())-1;
+  std::string_view::const_reverse_iterator right =
+    std::find_if_not(s.rbegin(), s.rend(), pred);
+  std::size_t j = std::distance(right, s.rend()) - 1;
 
-  return s.substr(i, (j-i+1));
+  // copy happens here; this could return a view in the future
+  return std::string{s.substr(i, (j - i + 1))};
 }
 
 void split_string(
-  const std::string &s,
+  std::string_view s,
   char delim,
   std::vector<std::string> &result,
   bool strip,
@@ -54,42 +55,48 @@ void split_string(
     return;
   }
 
-  std::string::size_type n=s.length();
+  std::size_t n = s.length();
   INVARIANT(n > 0, "Empty string case should already be handled");
 
-  std::string::size_type start=0;
-  std::string::size_type i;
+  std::size_t start = 0;
+  std::size_t i;
 
   for(i=0; i<n; i++)
   {
     if(s[i]==delim)
     {
-      std::string new_s=s.substr(start, i-start);
+      // result owns std::strings rather than string_views: callers
+      // routinely pass a temporary as `s` (e.g. some_function()
+      // returning std::string), so the input may not outlive the
+      // result.
+      std::string new_s = std::string{s.substr(start, i - start)};
 
       if(strip)
         new_s=strip_string(new_s);
 
       if(!remove_empty || !new_s.empty())
-        result.push_back(new_s);
+        result.push_back(std::move(new_s));
 
       start=i+1;
     }
   }
 
-  std::string new_s=s.substr(start, n-start);
+  // result owns std::strings rather than string_views: see the
+  // comment above the first push_back in the loop.
+  std::string new_s = std::string{s.substr(start, n - start)};
 
   if(strip)
     new_s=strip_string(new_s);
 
   if(!remove_empty || !new_s.empty())
-    result.push_back(new_s);
+    result.push_back(std::move(new_s));
 
   if(!remove_empty && result.empty())
     result.push_back("");
 }
 
 void split_string(
-  const std::string &s,
+  std::string_view s,
   char delim,
   std::string &left,
   std::string &right,
@@ -102,31 +109,26 @@ void split_string(
 
   if(result.size() != 2)
   {
-    throw deserialization_exceptiont{"expected string '" + s +
-                                     "' to contain two substrings "
-                                     "delimited by " +
-                                     delim + " but has " +
-                                     std::to_string(result.size())};
+    throw deserialization_exceptiont{
+      "expected string '" + std::string{s} +
+      "' to contain two substrings "
+      "delimited by " +
+      delim + " but has " + std::to_string(result.size())};
   }
 
   left=result[0];
   right=result[1];
 }
 
-std::vector<std::string> split_string(
-  const std::string &s,
-  char delim,
-  bool strip,
-  bool remove_empty)
+std::vector<std::string>
+split_string(std::string_view s, char delim, bool strip, bool remove_empty)
 {
   std::vector<std::string> result;
   split_string(s, delim, result, strip, remove_empty);
   return result;
 }
 
-std::string trim_from_last_delimiter(
-  const std::string &s,
-  const char delim)
+std::string trim_from_last_delimiter(std::string_view s, const char delim)
 {
   std::string result;
   const size_t index=s.find_last_of(delim);
@@ -135,7 +137,7 @@ std::string trim_from_last_delimiter(
   return result;
 }
 
-std::string escape(const std::string &s)
+std::string escape(std::string_view s)
 {
   std::string result;
 
@@ -150,7 +152,7 @@ std::string escape(const std::string &s)
   return result;
 }
 
-std::string escape_non_alnum(const std::string &to_escape)
+std::string escape_non_alnum(std::string_view to_escape)
 {
   std::ostringstream escaped;
   for(auto &ch : to_escape)
@@ -172,11 +174,12 @@ std::string escape_non_alnum(const std::string &to_escape)
   }
   return escaped.str();
 }
-std::string capitalize(const std::string &str)
+
+std::string capitalize(std::string_view s)
 {
-  if(str.empty())
-    return str;
-  std::string capitalized = str;
+  if(s.empty())
+    return std::string{};
+  std::string capitalized = std::string{s}; // copy
   capitalized[0] = toupper(capitalized[0]);
   return capitalized;
 }

src/util/string_utils.h

@@ -11,14 +11,15 @@ Author: Daniel Poetzl
 #define CPROVER_UTIL_STRING_UTILS_H
 
 #include <string>
+#include <string_view>
 #include <vector>
 
-std::string strip_string(const std::string &s);
+std::string strip_string(std::string_view s);
 
-std::string capitalize(const std::string &str);
+std::string capitalize(std::string_view s);
 
 void split_string(
-  const std::string &s,
+  std::string_view s,
   char delim,
   std::string &left,
   std::string &right,
@@ -34,14 +35,12 @@ void split_string(
 ///   This is applied after strip so whitespace only elements will be removed if
 ///   both are set to true.
 std::vector<std::string> split_string(
-  const std::string &s,
+  std::string_view s,
   char delim,
   bool strip = false,
   bool remove_empty = false);
 
-std::string trim_from_last_delimiter(
-  const std::string &s,
-  const char delim);
+std::string trim_from_last_delimiter(std::string_view s, const char delim);
 
 /// Prints items to an stream, separated by a constant delimiter
 /// \tparam It: An iterator type
@@ -97,13 +96,13 @@ join_strings(Stream &&os, const It b, const It e, const Delimiter &delimiter)
 
 /// Generic escaping of strings; this is not meant to be a particular
 /// programming language.
-std::string escape(const std::string &);
+std::string escape(std::string_view s);
 
 /// Replace non-alphanumeric characters with `_xx` escapes, where xx are hex
 /// digits. Underscores are replaced by `__`.
 /// \param to_escape: string to escape
 /// \return string with non-alphanumeric characters escaped
-std::string escape_non_alnum(const std::string &to_escape);
+std::string escape_non_alnum(std::string_view to_escape);
 
 /// Wrap line at spaces to not extend past the right margin, and include given
 /// padding with spaces to the left

unit/Makefile

@@ -207,10 +207,12 @@ SRC += analyses/ai/ai.cpp \
        util/string2int.cpp \
        util/structured_data.cpp \
        util/string_utils/capitalize.cpp \
+       util/string_utils/escape.cpp \
        util/string_utils/escape_non_alnum.cpp \
        util/string_utils/join_string.cpp \
        util/string_utils/split_string.cpp \
        util/string_utils/strip_string.cpp \
+       util/string_utils/trim_from_last_delimiter.cpp \
        util/string_utils/wrap_line.cpp \
        util/symbol_table.cpp \
        util/symbol.cpp \

unit/util/string_utils/capitalize.cpp

@@ -6,9 +6,12 @@ Author: Thomas Kiley
 
 \*******************************************************************/
 
-#include <testing-utils/use_catch.h>
 #include <util/string_utils.h>
 
+#include <testing-utils/use_catch.h>
+
+#include <string_view>
+
 TEST_CASE("capitalize", "[core][util][string_utils]")
 {
   REQUIRE(capitalize("") == "");
@@ -18,3 +21,14 @@ TEST_CASE("capitalize", "[core][util][string_utils]")
   REQUIRE(capitalize("abc def") == "Abc def");
   REQUIRE(capitalize("1") == "1");
 }
+
+TEST_CASE(
+  "capitalize honours string_view length over a non-NUL-terminated buffer",
+  "[core][util][string_utils]")
+{
+  // Without a trailing NUL: a regression where the implementation
+  // walked the buffer until '\0' would read past the end.
+  const char buf[] = {'a', 'b', 'c', 'X', 'Y'};
+  std::string_view sv{buf, 3};
+  REQUIRE(capitalize(sv) == "Abc");
+}

unit/util/string_utils/escape.cpp

@@ -0,0 +1,37 @@
+/*******************************************************************\
+
+Module: Unit tests of escape
+
+Author: Diffblue Ltd.
+
+\*******************************************************************/
+
+/// \file
+/// escape Unit Tests
+
+#include <util/string_utils.h>
+
+#include <testing-utils/use_catch.h>
+
+#include <string_view>
+
+TEST_CASE("escape", "[core][utils][string_utils][escape]")
+{
+  REQUIRE(escape("") == "");
+  REQUIRE(escape("abc") == "abc");
+  REQUIRE(escape("a\"b") == "a\\\"b");
+  REQUIRE(escape("a\\b") == "a\\\\b");
+  // characters other than `"` and `\` are passed through unchanged
+  REQUIRE(escape("a'b") == "a'b");
+}
+
+TEST_CASE(
+  "escape honours string_view length over a non-NUL-terminated buffer",
+  "[core][utils][string_utils][escape]")
+{
+  // Without a trailing NUL: a regression where the implementation
+  // walked the buffer until '\0' would read past the end.
+  const char buf[] = {'a', '"', 'b', 'X', 'Y'};
+  std::string_view sv{buf, 3};
+  REQUIRE(escape(sv) == "a\\\"b");
+}