Skip to content

[RFC] Make std.algorithm.searching.skipOver an eponymous template to allow partial instantiation #5576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wilzbach merged 2 commits into from
Dec 2, 2017
Merged

[RFC] Make std.algorithm.searching.skipOver an eponymous template to allow partial instantiation #5576

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a2a2da
Make std.algorithm.searching.skipOver an eponymous template
wilzbach Jul 7, 2017
7fb31ac
Add unittest for partial instantiation
wilzbach Jul 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 75 additions & 84 deletions std/algorithm/searching.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3913,72 +3913,81 @@ Do nothing if there is no match.
Params:
pred = The predicate that determines whether elements from each respective
range match. Defaults to equality $(D "a == b").
*/
template skipOver(alias pred = "a == b")
Copy link
Contributor Author

@wilzbach wilzbach Jul 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the eponymous pattern also used by other methods in this module (e.g. canFind, all or any).

{
/**
r1 = The $(REF_ALTTEXT forward range, isForwardRange, std,range,primitives) to
move forward.
r2 = The $(REF_ALTTEXT input range, isInputRange, std,range,primitives)
representing the initial segment of $(D r1) to skip over.
e = The element to match.

Returns:
true if the initial segment of $(D r1) matches $(D r2) or $(D pred) evaluates to true,
and $(D r1) has been advanced to the point past this segment; otherwise false, and
$(D r1) is left in its original position.
*/
bool skipOver(R1, R2)(ref R1 r1, R2 r2)
if (isForwardRange!R1 && isInputRange!R2
&& is(typeof(r1.front == r2.front)))
{
static if (is(typeof(r1[0 .. $] == r2) : bool)
&& is(typeof(r2.length > r1.length) : bool)
&& is(typeof(r1 = r1[r2.length .. $])))
Returns:
true if the initial segment of $(D r1) matches $(D r2) or $(D pred) evaluates to true,
and $(D r1) has been advanced to the point past this segment; otherwise false, and
$(D r1) is left in its original position.
*/
bool skipOver(R1, R2)(ref R1 r1, R2 r2)
if (is(typeof(binaryFun!pred(r1.front, r2.front))) &&
Copy link
Member

@andralex andralex Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ifTestable (not related to this PR but can go with it because code disobeying that doesn't compile anyway)

isForwardRange!R1 &&
isInputRange!R2)
{
if (r2.length > r1.length || r1[0 .. r2.length] != r2)
static if (is(typeof(pred) : string) && pred == "a == b"
Copy link
Contributor Author

@wilzbach wilzbach Jul 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware that @andralex isn't a huge fan of this pattern, but without this, we can't have a default pred for skipOver and thus would need to duplicate all overloads (as previously done).

Copy link
Member

@andralex andralex Jul 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If alternatives are a lot of aggravationm, then fine. But seeing template skipOver is already a sign that something has gotten too complicated...

Copy link
Contributor Author

@wilzbach wilzbach Jul 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But seeing template skipOver is already a sign that something has gotten too complicated...

Making skipOver is fully on purpose and what this entire informal RFC PR is about.

&& is(typeof(r1[0 .. $] == r2) : bool)
&& is(typeof(r2.length > r1.length) : bool)
&& is(typeof(r1 = r1[r2.length .. $])))
{
return false;
if (r2.length > r1.length || r1[0 .. r2.length] != r2)
{
return false;
}
r1 = r1[r2.length .. $];
return true;
}
else
{
static if (hasLength!R1 && hasLength!R2)
{
// Shortcut opportunity!
if (r2.length > r1.length)
return false;
}
auto r = r1.save;
while (!r2.empty && !r.empty && binaryFun!pred(r.front, r2.front))
{
r.popFront();
r2.popFront();
}
if (r2.empty)
r1 = r;
return r2.empty;
}
r1 = r1[r2.length .. $];
return true;
}
else
{
return skipOver!((a, b) => a == b)(r1, r2);
}
}

/// Ditto
bool skipOver(alias pred, R1, R2)(ref R1 r1, R2 r2)
if (is(typeof(binaryFun!pred(r1.front, r2.front))) &&
isForwardRange!R1 &&
isInputRange!R2)
{
static if (hasLength!R1 && hasLength!R2)
/// Ditto
bool skipOver(R)(ref R r1)
if (isForwardRange!R &&
ifTestable!(typeof(r1.front), unaryFun!pred))
{
// Shortcut opportunity!
if (r2.length > r1.length)
if (r1.empty || !unaryFun!pred(r1.front))
return false;

do
r1.popFront();
while (!r1.empty && unaryFun!pred(r1.front));
return true;
}
auto r = r1.save;
while (!r2.empty && !r.empty && binaryFun!pred(r.front, r2.front))

/// Ditto
bool skipOver(R, E)(ref R r, E e)
if (is(typeof(binaryFun!pred(r.front, e))) && isInputRange!R)
Copy link
Member

@andralex andralex Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

predicate should be ifTestable

{
if (r.empty || !binaryFun!pred(r.front, e))
return false;
r.popFront();
r2.popFront();
return true;
}
if (r2.empty)
r1 = r;
return r2.empty;
}

/// Ditto
bool skipOver(alias pred, R)(ref R r1)
if (isForwardRange!R &&
ifTestable!(typeof(r1.front), unaryFun!pred))
{
if (r1.empty || !unaryFun!pred(r1.front))
return false;

do
r1.popFront();
while (!r1.empty && unaryFun!pred(r1.front));
return true;
}

///
Expand Down Expand Up @@ -4009,40 +4018,6 @@ if (isForwardRange!R &&
assert(s4.skipOver!isWhite && s3.empty);
}

/**
Skip over the first element of the given range if it matches the given element,
otherwise do nothing.

Params:
pred = The predicate that determines whether an element from the range
matches the given element.

r = The $(REF_ALTTEXT input range, isInputRange, std,range,primitives) to skip
over.

e = The element to match.

Returns:
true if the first element matches the given element according to the given
predicate, and the range has been advanced by one element; otherwise false, and
the range is left untouched.
*/
bool skipOver(R, E)(ref R r, E e)
if (isInputRange!R && is(typeof(r.front == e) : bool))
{
return skipOver!((a, b) => a == b)(r, e);
}

/// Ditto
bool skipOver(alias pred, R, E)(ref R r, E e)
if (is(typeof(binaryFun!pred(r.front, e))) && isInputRange!R)
{
if (r.empty || !binaryFun!pred(r.front, e))
return false;
r.popFront();
return true;
}

///
@safe unittest
{
Expand All @@ -4064,6 +4039,22 @@ if (is(typeof(binaryFun!pred(r.front, e))) && isInputRange!R)
assert(!s2.skipOver('a'));
}

/// Partial instantiation
@safe unittest
{
import std.ascii : isWhite;
import std.range.primitives : empty;

alias whitespaceSkiper = skipOver!isWhite;

auto s2 = "\t\tvalue";
auto s3 = "";
auto s4 = "\t\t\t";
assert(whitespaceSkiper(s2) && s2 == "value");
assert(!whitespaceSkiper(s2));
assert(whitespaceSkiper(s4) && s3.empty);
}

/**
Checks whether the given
$(REF_ALTTEXT input range, isInputRange, std,range,primitives) starts with (one
Expand Down