[PATCH] D24842: Add StringRef {take, drop} x {_while, _until}

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a reviewer: sanjoy.
sanjoy added a comment.

lgtm modulo minor comments


================
Comment at: include/llvm/ADT/StringRef.h:280
@@ -278,1 +279,3 @@
 
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    LLVM_ATTRIBUTE_UNUSED_RESULT
----------------
Docs?

================
Comment at: include/llvm/ADT/StringRef.h:283
@@ +282,3 @@
+    size_t find_if(function_ref<bool(char)> F, size_t From = 0) const {
+      StringRef S = drop_front(From);
+      while (!S.empty()) {
----------------
Do you know if clang generates a zero-abstraction-overhead loop for this code?  If it doesn't, it is an LLVM bug. :)

================
Comment at: include/llvm/ADT/StringRef.h:292
@@ +291,3 @@
+
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    size_t find_if_not(function_ref<bool(char)> F, size_t From = 0) const {
----------------
No ` LLVM_ATTRIBUTE_UNUSED_RESULT` here?

================
Comment at: include/llvm/ADT/StringRef.h:293
@@ +292,3 @@
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    size_t find_if_not(function_ref<bool(char)> F, size_t From = 0) const {
+      return find_if([F](char c) { return !F(c); }, From);
----------------
Docs?

================
Comment at: include/llvm/ADT/StringRef.h:517
@@ -498,1 +516,3 @@
 
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    LLVM_ATTRIBUTE_UNUSED_RESULT
----------------
Docs?

Applies to all of the new functions you added.

================
Comment at: unittests/ADT/StringRefTest.cpp:918
@@ +917,3 @@
+
+  Test = "";
+  Taken = Test.drop_while([](char c) { return true; });
----------------
I'd use a separate variable for this:

```
StringRef EmptyStr = "";
```

Same comment for `TakeWhileUntil`.


https://reviews.llvm.org/D24842