[llvm-commits] [PATCH][Support/StringRef] Add rtrim.

[Chandler Carruth]

Minor comments below. Feel free to commit once addressed (even if that is
"no").

diff --git a/include/llvm/ADT/StringRef.h b/include/llvm/ADT/StringRef.h
index 76ba66e..b1f376b 100644
--- a/include/llvm/ADT/StringRef.h
+++ b/include/llvm/ADT/StringRef.h
@@ -292,6 +292,16 @@ namespace llvm {
     /// Note: O(size() + Chars.size())
     size_type find_last_of(StringRef Chars, size_t From = npos) const;

+    /// find_last_not_of - Find the last character in the string that is
not
+    /// \arg C, or npos if not found.
+    size_type find_last_not_of(char C, size_t From = npos) const ;

extraneous space before the ';'.

+
+    /// find_last_not_of - Find the last character in the string that is
not in
+    /// \arg Chars, or npos if not found.
+    ///
+    /// Note: O(size() + Chars.size())
+    size_type find_last_not_of(StringRef Chars, size_t From = npos) const ;

extraneous space before the ';'.

+
     /// @}
     /// @name Helpful Algorithms
     /// @{
@@ -480,6 +490,18 @@ namespace llvm {
       return std::make_pair(slice(0, Idx), slice(Idx+1, npos));
     }

+    StringRef ltrim(StringRef Chars) const {
+      return drop_front(std::min(Length, find_first_not_of(Chars)));

find_first_not_of supports a single 'char' input, but this doesn't -- any
particular reason?

Do we want to provide a whitespace trimming default argument? (I'm
ambivalent about it)

+    }
+
+    StringRef rtrim(StringRef Chars) const {
+      return drop_back(Length - std::min(Length, find_last_not_of(Chars) +
1));
+    }
+
+    StringRef trim(StringRef Chars) const {
+      return ltrim(Chars).rtrim(Chars);
+    }
+

+/// find_last_not_of - Find the last character in the string that is not in
+/// \arg Chars, or npos if not found.
+///
+/// Note: O(size() + Chars.size())
+StringRef::size_type StringRef::find_last_not_of(StringRef Chars,
+                                                 size_t From) const {
+  std::bitset<1 << CHAR_BIT> CharBits;
+  for (size_type i = 0; i != Chars.size(); ++i)
+    CharBits.set((unsigned char)Chars[i]);
+

cache the size in a loop variable here.

+TEST(StringRefTest, Trim) {
+  StringRef Str0("hello");
+  StringRef Str1(" hello ");
+  StringRef Str2("  hello  ");
+
+  EXPECT_EQ(StringRef("hello"), Str0.rtrim(" "));
+  EXPECT_EQ(StringRef(" hello"), Str1.rtrim(" "));
+  EXPECT_EQ(StringRef("  hello"), Str2.rtrim(" "));
+  EXPECT_EQ(StringRef("hello"), Str0.ltrim(" "));
+  EXPECT_EQ(StringRef("hello "), Str1.ltrim(" "));
+  EXPECT_EQ(StringRef("hello  "), Str2.ltrim(" "));
+  EXPECT_EQ(StringRef("hello"), Str0.trim(" "));
+  EXPECT_EQ(StringRef("hello"), Str1.trim(" "));
+  EXPECT_EQ(StringRef("hello"), Str2.trim(" "));
+}
+

We can do a lot better testing than this. Cases that I would find
interesting to pedantically check in the test:

""
"  "
"  \0  "
"\0\0"

StringRef("\0\0x\0\0").trim('\0')

.trim("aaaaa")


-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120511/79c8f4b8/attachment.html>