Minor comments below. Feel free to commit once addressed (even if that is "no").<div><br></div><div><div>diff --git a/include/llvm/ADT/StringRef.h b/include/llvm/ADT/StringRef.h</div><div>index 76ba66e..b1f376b 100644</div>
<div>--- a/include/llvm/ADT/StringRef.h</div><div>+++ b/include/llvm/ADT/StringRef.h</div><div>@@ -292,6 +292,16 @@ namespace llvm {</div><div>     /// Note: O(size() + Chars.size())</div><div>     size_type find_last_of(StringRef Chars, size_t From = npos) const;</div>
<div> </div><div>+    /// find_last_not_of - Find the last character in the string that is not</div><div>+    /// \arg C, or npos if not found.</div><div>+    size_type find_last_not_of(char C, size_t From = npos) const ;</div>
<div><br></div><div>extraneous space before the ';'.</div><div><br></div><div>+</div><div>+    /// find_last_not_of - Find the last character in the string that is not in</div><div>+    /// \arg Chars, or npos if not found.</div>
<div>+    ///</div><div>+    /// Note: O(size() + Chars.size())</div><div>+    size_type find_last_not_of(StringRef Chars, size_t From = npos) const ;</div><div><br></div><div>extraneous space before the ';'.</div>
<div><br></div><div>+</div><div>     /// @}</div><div>     /// @name Helpful Algorithms</div><div>     /// @{</div><div>@@ -480,6 +490,18 @@ namespace llvm {</div><div>       return std::make_pair(slice(0, Idx), slice(Idx+1, npos));</div>
<div>     }</div><div> </div><div>+    StringRef ltrim(StringRef Chars) const {</div><div>+      return drop_front(std::min(Length, find_first_not_of(Chars)));</div><div><br></div><div>find_first_not_of supports a single 'char' input, but this doesn't -- any particular reason?</div>
<div><br></div><div>Do we want to provide a whitespace trimming default argument? (I'm ambivalent about it)</div><div><br></div><div>+    }</div><div>+</div><div>+    StringRef rtrim(StringRef Chars) const {</div><div>
+      return drop_back(Length - std::min(Length, find_last_not_of(Chars) + 1));</div><div>+    }</div><div>+</div><div>+    StringRef trim(StringRef Chars) const {</div><div>+      return ltrim(Chars).rtrim(Chars);</div>
<div>+    }</div><div>+</div><div><br></div><div><div>+/// find_last_not_of - Find the last character in the string that is not in</div><div>+/// \arg Chars, or npos if not found.</div><div>+///</div><div>+/// Note: O(size() + Chars.size())</div>
<div>+StringRef::size_type StringRef::find_last_not_of(StringRef Chars,</div><div>+                                                 size_t From) const {</div><div>+  std::bitset<1 << CHAR_BIT> CharBits;</div><div>
+  for (size_type i = 0; i != Chars.size(); ++i)</div><div>+    CharBits.set((unsigned char)Chars[i]);</div><div>+</div></div><div><br></div><div>cache the size in a loop variable here.</div><div><br></div><div><div>+TEST(StringRefTest, Trim) {</div>
<div>+  StringRef Str0("hello");</div><div>+  StringRef Str1(" hello ");</div><div>+  StringRef Str2("  hello  ");</div><div>+</div><div>+  EXPECT_EQ(StringRef("hello"), Str0.rtrim(" "));</div>
<div>+  EXPECT_EQ(StringRef(" hello"), Str1.rtrim(" "));</div><div>+  EXPECT_EQ(StringRef("  hello"), Str2.rtrim(" "));</div><div>+  EXPECT_EQ(StringRef("hello"), Str0.ltrim(" "));</div>
<div>+  EXPECT_EQ(StringRef("hello "), Str1.ltrim(" "));</div><div>+  EXPECT_EQ(StringRef("hello  "), Str2.ltrim(" "));</div><div>+  EXPECT_EQ(StringRef("hello"), Str0.trim(" "));</div>
<div>+  EXPECT_EQ(StringRef("hello"), Str1.trim(" "));</div><div>+  EXPECT_EQ(StringRef("hello"), Str2.trim(" "));</div><div>+}</div><div>+</div></div><div><br></div><div>We can do a lot better testing than this. Cases that I would find interesting to pedantically check in the test:</div>
<div><br></div><div>""</div><div>"  "</div><div>"  \0  "</div><div>"\0\0"</div><div><br></div><div>StringRef("\0\0x\0\0").trim('\0')</div><div><br></div><div>.trim("aaaaa")</div>
<div><br></div><div><br></div><div>-Chandler</div></div>