Add Specializations for SortedRange

[nordlow]

Add new predicate function std.range.primitives.isSortedRange: DONE

Add specializations for SortedRange in the followings algorithms:

• sort: just return input argument if pred parameter matches: DONE
• sort-alikes: just return input argument if pred parameter matches:
• isSorted: return true: DONE
• find and alikes, should do some kind of binary search. These could take an extra param SearchPolicy for sorted ranges that defaults to binary search.
• Some of the functions that return a sub-range or a mutated range could probably be returning sorted ranges as well if its input is a sorted range, remove, strip and split at least could.
• minPos: return input as is, if pred matches: DONE

In compliance with C++ STL also add

• minElement: O(1) for SortedRange, O(n) otherwise: DONE
• maxElement: O(1) for SortedRange!BidirectionalRange, O(n) otherwise: DONE
• minmaxElement: O(1) for SortedRange!BidirectionalRange, O(n) otherwise.

See also: http://forum.dlang.org/post/yenezfjjteokzyvgmzcf@forum.dlang.org
See also: https://issues.dlang.org/show_bug.cgi?id=11667
See also: http://forum.dlang.org/post/mqskao$28vh$1@digitalmars.com

See also: http://en.cppreference.com/w/cpp/algorithm/min_element

[DmitryOlshansky]

O(2)

Hilarious. It's sill O(1), the number just means it's some constant factor times 1 so it doesn't matter if this number is 1, 2, 1000 since multiplier in front of it is unspecified.

[DmitryOlshansky]

std.range.primitives.isSortedRange

I recall what the problem we had with it was - literally things like SortedRange!(int[], binaryFun!" a < b") and SortedRange!(int[], (a,b) => a < b) and SortedRange!(int[], (b,c) => b < c) are distinct types b/c respective predicates are not identical.

It doesn't block anything but should be taken into consideration, a compiler ought to try and match lambas with identical bodies at least token-wise identical.

[nordlow]

Hilarious. It's sill O(1)

I kind of the new that ;)

[nordlow]

1. Any suggestions on how to compare pred lambdas?

For instance

static assert(is(typeof(binaryFun!"a<b") ==
                        typeof(binaryFun!"a<b")));
static assert(is(typeof(binaryFun!"a<b") ==
                        typeof(binaryFun!"a < b")));

both passes but

static assert(!is(typeof(binaryFun!"a<b") ==
                         typeof(binaryFun!"a>b")));

fails. What to do about this?...should we extract and reuse the CT-parsing logic of "a < b" which I presume is happening in std.functional?

It doesn't block anything but should be taken into consideration, a compiler ought to try and match lambas with identical bodies at least token-wise identical.

Is this lambda-comparison just a wish or is there something that can be used already?

1. Should minElement, maxElement and minmaxElement return a range instead? Or perhaps a Nullable!ElementType!R. The corresponding C++ algorithms all return iterators.
2. Should we add a specialization for reduce aswell so that the user isn't force to use minElement to get these optimizations?

[JakobOvrum]

The return type is Range for all paths; explicit return type is better for documented functions as it provides important information to readers.

[nordlow]

Done.

[JakobOvrum]

Functions that return subranges often already them as the same type as the whole range; this already works with SortedRange.

Comparing predicates has been discussed a lot before, particularly in the context of string lambdas. One such thread:
http://forum.dlang.org/post/jnlqesrwxfekdsxjerlp@forum.dlang.org (Sorry, I have a hard time digging up the older ones)

[quickfur]

I suggest to break this PR up into smaller pieces so that it's easier to review, and the non-controversial parts can be merged first while we work on the other parts. Otherwise this will go very slowly and take too long to get merged.

[quickfur]

As a start, I'd say do a separate PR per overload set, as a rough guideline. I think that's much easier to review, and safe to merge piecemeal.

[quickfur]

ping @nordlow
Let's move forward with this?

[nordlow]

@quickfur Should I start with an initial pull for the new std.range.primitives.isSortedRange eventhough I have not not found an ideal way to detect which pred argument that was given as template argument to isSortedRange?

One way to move forward with this PR is to restrict logic to a specific set of expressions such as

• "a < b"
• "a > b"
• "a <= b"
• "a >= b"

which should cover most uses. All other inputs should trigger a compile-time error with a nice descriptive message.

[quickfur]

Hmm. On second thoughts, there is no good way to compare two lambdas (whether string or function literal -- the latter because the compiler has no implementation of such a comparison). So there is no good solution for moving this PR forward.

The problem is that comparing two arbitrary lambdas is, in the most general case, uncomputable. For most practical purposes, though, we can reduce the problem to something tractable by ignoring non-trivial equivalences such as (a < b) == !(a >= b), and just looking at AST equivalence. Of course, that also needs some further restriction, since a < b+x may mean different things if x is bound to the surrounding context of the lambda, so if they are bound to two different contexts they should not compare as equal.

But in any case, this requires compiler support, and I agree with the sentiment that we should not promote string lambdas anymore so it's kinda pointless to support these functions just for string lambdas. Let's wait until we have a DIP on how to implement lambda comparisons.

[MetaLang]

Of course, that also needs some further restriction, since a < b+x may mean different things if x is bound to the surrounding context of the lambda, so if they are bound to two different contexts they should not compare as equal.

I'm not sure if you're saying that this is bad, but I'd consider that to be a pretty good thing.

[quickfur]

I was just thinking aloud. Obviously, it would be very bad if the two lambdas in the following code compared equal!

auto sortedRange1(int[] a)
{
    auto x = 1;
    return a.sort!((a,b) => a < x*b);
}
auto sortedRange2(int[] a)
{
    auto x =  -1;
    return a.sort!((a,b) => a < x*b);
}

[sigod]

I have a question: Why not to add specializations directly into SortedRange?

[nordlow]

That might be cleaner, thanks to UFCS and because isSortedRange doesn't have to be imported.

AFAIK: Lambda comparison problem will remain, though.

[quickfur]

Actually, if the specializations are added as member functions of SortedRange, then we don't need isSortedRange and lambda comparisons, because the member functions will have direct access to the sorting predicate and the range can safely be assumed to be sorted (since otherwise it wouldn't be a SortedRange to begin with).

Given the current state of things, that could potentially be the better way to go right now.

[nordlow]

Ok, I'll look into it.

I guess we should be begin with calls such as:

• sr.sort
• sr.find(element)
• sr.isSorted
• sr.minPos

where sr is an instance of a SortedRange.

I guess find doesn't need the `SearchPolicy´ argument then, right?

Anymore low-hanging fruits?

But note that this, of course, only works for calls to sort, isSorted, minPos without explicit lambda-argument pred. But I guess that's ok, right?

[sigod]

then we don't need ... lambda comparisons

There's slight problem. For example, if range were sorted with "a < b" and user wants to sort it again with "a > b". Ideally it's just a call to retro.

[sigod]

Anymore low-hanging fruits?

minCount, probably.

[quickfur]

@sigod We can't possibly know about all these special cases. It would be up to user code to detect such a case and use retro instead of sort. (Of course, ideally, sort should be using an algorithm that behaves pretty closely to calling retro when the incoming range is sorted in reverse order.) But there's also the question of how common is it to need to call sort on a sorted range with the reverse predicate, to justify including this special case in the standard library. If this is a rare use case, it doesn't justify the cost of additional complexity in SortedRange.

(Not to mention that determining whether two predicates are the opposite of each other, generally speaking, is uncomputable. It's easy to detect built-in operators < and >, but what if the user type implements a non-trivial opCmp? What if opCmp is a partial order, rather than a total order? Etc.)

[sigod]

(Not to mention that determining whether two predicates are the opposite of each other, generally speaking, is uncomputable. It's easy to detect built-in operators < and >, but what if the user type implements a non-trivial opCmp? What if opCmp is a partial order, rather than a total order? Etc.)

This didn't occurred to me. Then I think such detection is out of question.

But note that this, of course, only works for calls to sort, isSorted, minPos without explicit lambda-argument pred. But I guess that's ok, right?

We probably could easily add support for the same pred with which SortedRange were constructed.

[wilzbach]

@nordlow {min,max}Element are now in Phobos (crowd goes woohoo!). I actually didn't know you also had them in this PR ;-)
Thus could you please remove them from your PR?

Actually, if the specializations are added as member functions of SortedRange, then we don't need isSortedRange and lambda comparisons

What else was blocking this PR?

[nordlow]

But we still need SortedRange-specializations for {min,max,minmax}Element, right? The default implementations of them don't care about sortedness (SortedRange).

Should these overloads be implemented as free functions take a SortedRange as argument or via SortedRange-members?

I'll restrict the overloads in this pull to only operate on SortedRange with a predicate that matches the predicate of minElement.

[wilzbach]

I'll restrict the overloads in this pull to only operate on SortedRange with a predicate that matches the predicate of minElement.

1. Please use static if - we should put such details to the implementation. Have a look at the post by big boss.

2. You should also have a look at std.algorithm.search: expose extremum #4257 - it's an "optimization" for the default case of an empty lambda.

3. How do you know that the mapping function if given and the sorting pred match? -> imho we can only make the static if if no mapping function is used (see 2))

[sigod]

Should these overloads be implemented as free functions take a SortedRange as argument or via SortedRange-members?

I vote for SortedRange-members. It's easier and cleaner.

[wilzbach]

I vote for SortedRange-members. It's easier and cleaner.

How do you avoid that SortedRanges match the other template? Won't you have to add something like !isSortedRange!R

[sigod]

How do you avoid that SortedRanges match the other template?

It's intended for SortedRange-members to "override" some of the common templates.

Won't you have to add something like !isSortedRange!R

I hope not.

[nordlow]

But to support overloading of minElement(Range, Seed) don't we need to override behaviour through a free function overload

minElement(R)(R r)
    if (isSortedRange!R)
{ ... }

?

So if is defined as member function in SortedRange a call minElement(sortedRange) won't pick the overload. or does D's UFCS work bothways? I don't remember. I only make use of it as: r.f(...) => f(r, ...), not vice versa.

[sigod]

minElement(Range, Seed)

Yes, for such use we need free function overload. To be honest I didn't consider it. I always use UFCS.

or does D's UFCS work bothways?

No. Only f(r, ...) => r.f(...).

[nordlow]

So, AFAICT, the only current solution that harmonizes with Phobos' own {min,max}Element is to provide specialization via member-call syntax only then, right? I hope I'm wrong...

Update: Can't we still use if (isInstanceOf(SortedRange, R) as restriction for free function overload solution?

[sigod]

So, AFAICT, the only current solution is to provide specialization is via member-call syntax then, right?

That or we need to deal with comparison of predicates. I'd go with former.

[JackStouffer]

Closing due to inactivity. @nordlow If you want to reopen this and continue working on it, please let me know.