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.

llvm/trunk/include/llvm/ADT/ArrayRef.h

public:
 
     /// \brief Drop the last \p N elements of the array.
     LLVM_ATTRIBUTE_UNUSED_RESULT
     ArrayRef<T> drop_back(size_t N = 1) const {
       assert(size() >= N && "Dropping more elements than exist");
       return slice(0, size() - N);
     }
 
+    /// \brief Keep the first \p N elements of the array.
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    ArrayRef<T> keep_front(size_t N = 1) const {
+      if (N >= size())
+        return *this;
+      return drop_back(size() - N);
+    }
+
+    /// \brief Keep the last \p N elements of the array.
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    ArrayRef<T> keep_back(size_t N = 1) const {
+      if (N >= size())
+        return *this;
+      return drop_front(size() - N);
+    }
+
     /// @}
     /// @name Operator Overloads
     /// @{
     const T &operator[](size_t Index) const {
       assert(Index < Length && "Invalid index!");
       return Data[Index];
     }
 
public:
     }
 
     LLVM_ATTRIBUTE_UNUSED_RESULT
     MutableArrayRef<T> drop_back(size_t N = 1) const {
       assert(this->size() >= N && "Dropping more elements than exist");
       return slice(0, this->size() - N);
     }
 
+    /// \brief Drop everything but the first \p N elements of the array.
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    MutableArrayRef<T> keep_front(size_t N = 1) const {
+      if (N >= this->size())
+        return *this;
+      return drop_back(size() - N);
+    }
+
+    /// \brief Drop everything but the last \p N elements of the array.
+    LLVM_ATTRIBUTE_UNUSED_RESULT
+    MutableArrayRef<T> keep_back(size_t N = 1) const {
+      if (N >= this->size())
+        return *this;
+      return drop_front(size() - N);
+    }
+
     /// @}
     /// @name Operator Overloads
     /// @{
     T &operator[](size_t Index) const {
       assert(Index < this->size() && "Invalid index!");
       return data()[Index];
     }
   };

llvm/trunk/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 take_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 take_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);
     }

llvm/trunk/unittests/ADT/ArrayRefTest.cpp

TEST(ArrayRefTest, DropFront) {
   ArrayRef<int> AR2(&TheNumbers[2], AR1.size() - 2);
   EXPECT_TRUE(AR1.drop_front(2).equals(AR2));
 
   // Check that drop_front accepts size_t-sized numbers.
   ArrayRef<char> AR3((const char *)0x10000, SIZE_MAX - 0x10000);
   EXPECT_EQ(1U, AR3.drop_front(AR3.size() - 1).size());
 }
 
+TEST(ArrayRefTest, KeepBack) {
+  static const int TheNumbers[] = {4, 8, 15, 16, 23, 42};
+  ArrayRef<int> AR1(TheNumbers);
+  ArrayRef<int> AR2(AR1.end() - 1, 1);
+  EXPECT_TRUE(AR1.keep_back().equals(AR2));
+}
+
+TEST(ArrayRefTest, KeepFront) {
+  static const int TheNumbers[] = {4, 8, 15, 16, 23, 42};
+  ArrayRef<int> AR1(TheNumbers);
+  ArrayRef<int> AR2(AR1.data(), 2);
+  EXPECT_TRUE(AR1.keep_front(2).equals(AR2));
+}
+
 TEST(ArrayRefTest, Equals) {
   static const int A1[] = {1, 2, 3, 4, 5, 6, 7, 8};
   ArrayRef<int> AR1(A1);
   EXPECT_TRUE(AR1.equals({1, 2, 3, 4, 5, 6, 7, 8}));
   EXPECT_FALSE(AR1.equals({8, 1, 2, 4, 5, 6, 6, 7}));
   EXPECT_FALSE(AR1.equals({2, 4, 5, 6, 6, 7, 8, 1}));
   EXPECT_FALSE(AR1.equals({0, 1, 2, 4, 5, 6, 6, 7}));
   EXPECT_FALSE(AR1.equals({1, 2, 42, 4, 5, 6, 7, 8}));

llvm/trunk/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, Take) {
+  StringRef Test("StringRefTest::Take");
+
+  StringRef Taken = Test.take_front(5);
+  EXPECT_EQ(Taken, "Strin");
+
+  Taken = Test.take_back(5);
+  EXPECT_EQ(Taken, ":Take");
+
+  Taken = Test.take_front(Test.size());
+  EXPECT_EQ(Taken, Test);
+
+  Taken = Test.take_back(Test.size());
+  EXPECT_EQ(Taken, Test);
+
+  Taken = Test.take_front(0);
+  EXPECT_TRUE(Taken.empty());
+
+  Taken = Test.take_back(0);
+  EXPECT_TRUE(Taken.empty());
+}
 
 } // end anonymous namespace