This is an archive of the discontinued LLVM Phabricator instance.

Add StringRef {take, drop} x {_while, _until}
ClosedPublic

Authored by zturner on Sep 22 2016, 1:47 PM.

Details

Summary
This adds support for some common functional style operations
that allow you to take or drop characters until a certain
condition is met.

Example:

assert(StringRef("ABCD2908XYZ".take_while([](char c) { return ::is_hexdigit(c); }) == "XYZ");

Diff Detail

Repository
rL LLVM

Event Timeline

zturner updated this revision to Diff 72208.Sep 22 2016, 1:47 PM
zturner retitled this revision from to Add StringRef {take, drop} x {_while, _until}.
zturner updated this object.
zturner added a subscriber: llvm-commits.
zturner updated this revision to Diff 72210.Sep 22 2016, 1:49 PM

Cleaned up the test. The lambda was unnecessary.

davide accepted this revision.Sep 22 2016, 2:00 PM
davide added a reviewer: davide.
davide added a subscriber: davide.

lgtm, I like llvm becoming more haskell'y.

This revision is now accepted and ready to land.Sep 22 2016, 2:00 PM
davide added inline comments.Sep 22 2016, 2:01 PM
include/llvm/ADT/StringRef.h
499–500 ↗(On Diff #72210)

Oh, and maybe if you can add a comment at the beginning of each function (e.g. as consume_front() already does) it would be good.

majnemer added inline comments.Sep 22 2016, 2:06 PM
include/llvm/ADT/StringRef.h
499–511 ↗(On Diff #72210)

Can you use function_ref instead?

sanjoy added a subscriber: sanjoy.Sep 22 2016, 10:10 PM

Have you considered adding StringRef(iterator, iterator) constructor, and replacing the explicit loops with std::find_if and std::find_if_not.

zturner updated this revision to Diff 72372.Sep 23 2016, 3:49 PM
zturner edited edge metadata.

Added find_if and made the drop and take functions use that.

zturner updated this revision to Diff 72374.Sep 23 2016, 3:53 PM

Last patch was wrong. Let's try this.

sanjoy accepted this revision.Sep 24 2016, 3:27 PM
sanjoy added a reviewer: sanjoy.

lgtm modulo minor comments

include/llvm/ADT/StringRef.h
280 ↗(On Diff #72374)

Docs?

283 ↗(On Diff #72374)

Do you know if clang generates a zero-abstraction-overhead loop for this code? If it doesn't, it is an LLVM bug. :)

292 ↗(On Diff #72374)

No LLVM_ATTRIBUTE_UNUSED_RESULT here?

293 ↗(On Diff #72374)

Docs?

517 ↗(On Diff #72374)

Docs?

Applies to all of the new functions you added.

unittests/ADT/StringRefTest.cpp
918 ↗(On Diff #72374)

I'd use a separate variable for this:

StringRef EmptyStr = "";

Same comment for TakeWhileUntil.

This revision was automatically updated to reflect the committed changes.