Add StringRef::keep_front() and StringRef::keep_back()

[zturner]

Summary

I've needed these methods time and time again, but always resort to using some incantation of slice(), substr(), or drop_front() / drop_back() with a length calculation.

While these methods are extremely simple, they can make certain operations very elegant, especially when you want to slice up a string in various ways and you want to chain the operations together.

[zturner]

Added the same functions for ArrayRef and MutableArrayRef

[majnemer]

Sounds like a useful API. The term I've heard of in other programing languages is "take", I think take_front and take_back might be a little nicer.

[zturner]

In D23965#527296, @majnemer wrote:

Sounds like a useful API. The term I've heard of in other programing languages is "take", I think take_front and take_back might be a little nicer.

take_front sounds ok, but take_back sounds awkward to me. I almost want to say "take what back?" If the convention was drop and rdrop, then take and rtake would sound good, but I'm not crazy about take_front and take_back. I could probably be convinced though if the majority prefers it.

[grandinj]

include/llvm/ADT/ArrayRef.h

340 ↗

(On Diff #69500)

this explanation can't be right

348 ↗

(On Diff #69500)

or this one

[zturner]

Fixed incorrect function descriptions.

[chfast]

In D23965#527297, @zturner wrote:

In D23965#527296, @majnemer wrote:

Sounds like a useful API. The term I've heard of in other programing languages is "take", I think take_front and take_back might be a little nicer.

take_front sounds ok, but take_back sounds awkward to me. I almost want to say "take what back?" If the convention was drop and rdrop, then take and rtake would sound good, but I'm not crazy about take_front and take_back. I could probably be convinced though if the majority prefers it.

So maybe take_first and take_last?

[zturner]

Adding a few more people for comments.

[amccarth]

I'm torn on the naming debate. I think take_back is awkward, and using first and last instead of front and back would make the new names inconsistent with drop_front and drop_back (and with STL container naming). I also see that point that keep is less precise.

Everything else LGTM.

[rnk]

I don't feel strongly, but I'll toss in a vote for take_front / take_back. "Keep" sounds like it's modifying the string in place.

include/llvm/ADT/StringRef.h

public:
     /// suffix (starting with \p Start) will be returned.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
     LLVM_ATTRIBUTE_UNUSED_RESULT
     StringRef substr(size_t Start, size_t N = npos) const {
       Start = std::min(Start, Length);
       return StringRef(Data + Start, std::min(N, Length - Start));
     }
 
+    /// Return a StringRef equal to 'this' but with only the first \p N
+    /// elements remaining.  If \p N is greater than the length of the
+    /// string, the entire string is returned.
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    StringRef keep_front(size_t N = 1) const {
+      if (N >= size())
+        return *this;
+      return drop_back(size() - N);
+    }
+
+    /// Return a StringRef equal to 'this' but with only the first \p N
+    /// elements remaining.  If \p N is greater than the length of the
+    /// string, the entire string is returned.
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    StringRef keep_back(size_t N = 1) const {
+      if (N >= size())
+        return *this;
+      return drop_front(size() - N);
+    }
+
     /// Return a StringRef equal to 'this' but with the first \p N elements
     /// dropped.
     LLVM_ATTRIBUTE_ALWAYS_INLINE
     LLVM_ATTRIBUTE_UNUSED_RESULT
     StringRef drop_front(size_t N = 1) const {
       assert(size() >= N && "Dropping more elements than exist");
       return substr(N);
     }

unittests/ADT/StringRefTest.cpp

TEST(StringRefTest, AllocatorCopy) {
   StringRef Str1c = Str1.copy(Alloc);
   StringRef Str2c = Str2.copy(Alloc);
   EXPECT_TRUE(Str1.equals(Str1c));
   EXPECT_NE(Str1.data(), Str1c.data());
   EXPECT_TRUE(Str2.equals(Str2c));
   EXPECT_NE(Str2.data(), Str2c.data());
 }
 
+TEST(StringRefTest, Drop) {
+  StringRef Test("StringRefTest::Drop");
+
+  StringRef Dropped = Test.drop_front(5);
+  EXPECT_EQ(Dropped, "gRefTest::Drop");
+
+  Dropped = Test.drop_back(5);
+  EXPECT_EQ(Dropped, "StringRefTest:");
+
+  Dropped = Test.drop_front(0);
+  EXPECT_EQ(Dropped, Test);
+
+  Dropped = Test.drop_back(0);
+  EXPECT_EQ(Dropped, Test);
+
+  Dropped = Test.drop_front(Test.size());
+  EXPECT_TRUE(Dropped.empty());
+
+  Dropped = Test.drop_back(Test.size());
+  EXPECT_TRUE(Dropped.empty());
+}
+
+TEST(StringRefTest, Keep) {
+  StringRef Test("StringRefTest::Keep");
+
+  StringRef Kept = Test.keep_front(5);
+  EXPECT_EQ(Kept, "Strin");
+
+  Kept = Test.keep_back(5);
+  EXPECT_EQ(Kept, ":Keep");
+
+  Kept = Test.keep_front(Test.size());
+  EXPECT_EQ(Kept, Test);
+
+  Kept = Test.keep_back(Test.size());
+  EXPECT_EQ(Kept, Test);
+
+  Kept = Test.keep_front(0);
+  EXPECT_TRUE(Kept.empty());
+
+  Kept = Test.keep_back(0);
+  EXPECT_TRUE(Kept.empty());
+}
 
 } // end anonymous namespace