Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improvements to hasSlicing #854

Merged
merged 9 commits into from

6 participants

@jmdavis
Collaborator

The primary goal of this pull request is to tighten up hasSlicing, since it's too lax. The changes to it are:

  1. For finite ranges, the result of opSlice must be implicitly convertible to the original to allow you to assume that the slice is reassignable to the original (except when the original was const or immutable, in which case it can be assigned to a new variable of the same type as the original). It's implicitly convertible rather than exact so that the slice can be tail-const.

  2. For infinite ranges, the result of opSlice must be the result of take or takeExactly (they now return the same type for infinite ranges).

  3. The result of opSlice is required to be a forward range rather than an input range. As slicing is essentially the same as calling save except for the fact that the result covers fewer elements, it makes no sense for the result to not be a forward range.

  4. The result of opSlice must define length as it makes no sense to have a slice with an unknown length. The original doesn't have to, even for finite ranges, because it really has no effect on opSlice whether the original defines length or not. But because the result must have length and must be assignable to the original (for finite ranges at least), it probably wouldn't make sense for the original not to have length if it's finite. It seemed pointless to check for it though.

Also, in order to facilitate using take or takeExactly for slicing infiinite ranges, take and takeExactly have been adjusted. The main difference is that now neither checks hasSlicing on infinite ranges (that causes a circular dependency which triggers bug# 8556), but takeExactly has also been adjusted to return the same type as take wherever possible (i.e. when the range is infinite or defines length).

@jmdavis
Collaborator

Of interesting note, I just figured out that these changes make it impossible to define slicing on a type which wraps an infinite range, because slicing the infinite range would mean getting the result of take, and the wrapper can't be instantiated with that type, because it differs from the original. I had to add a fix to map so that it didn't define slicing for infinite ranges, because this broke:

import std.algorithm;
import std.range;

void main()
{
    auto n2 = sequence!"n"(cast(size_t)1).map!"a";
}

So, this could definitely break some code involving infinite ranges, but if we want to be able to have the guarantee that a slice of a finite range is reassignable to the original (as code often assumes), I think that we're stuck. And not having that guarantee for finite ranges will definitely be problematic.

@monarchdodra
Collaborator

LGTM, regarding the finite ranges.

Regarding the infinite ones, I've been (silently) working on my "isDropable" development. It is looking good, and I think it can fix all the issues you have been running into. I've just been more busy fixing things then inserting new things.

isDropable would be definied as:
R is dropable if:
1. R is sliceable, in which case drop is equivalent to "r[n .. $]"
2. R explicitly defines the method popFrontN, (in which case one assumes that operation is o(1))

This allows two things:
1. Gives infinite ranges a natural way to "slice to end".
2. When operating on an infinite range, extracting a slice is the same as checking for "isDropable", and doing:
r.drop(i).take(length);

At this point, we could add a function in range akin to walkLength called extractSlice:
*This would extract a slice the natural way on sliceable ranges.
*Do the drop/take trick on infinite ranges that are dropable (with the added bonus of "i .. j" syntax, as opposed to "i, length")

using extractSlice would mean that the returned slice may not be re-assignable to the original range. IMO: This is a good thing: Users operating on infinite ranges should be aware of what they are doing. Using "extractSlice" is really a synonym to "I know I can't extract a re-assignable slice, but I know what I'm doing".

IMO, "hasSlicing" should mean that it is re-assignable to the original range, no exception.

Do you think it would be at all possible to post-pone the "infinite" part of this enhancement, so I can present my developed solution? Everything else, though, LGTM.

@jmdavis
Collaborator

Based on your description, I don't see how it solves any problems for slicing infinite ranges. In order for them to function the same way that slicing does with finite ranges, the slice must always assignable to the original, and that's never going to work, because the slice is inherently finite whereas the original is infinite.

The only thing from your proposal that's immediately relevant to this pull request as far as I can see is the idea that infinite ranges not ever be sliceable, which is arguably a good idea. I'm not particularly fond of the idea that they be sliceable, particularly since it screws up the guarantee that sliceable ranges are always reassignable to the original. But even if we take that tact, it doesn't really conflict with my changes. It just means that the slicing on infinite ranges is removed later. It wouldn't have any effect on finite ones. So, I think that we can look at doing what you propose later without affecting what I'm proposing here.

However, as much as I really like the idea of making infinite ranges non-sliceable, Andrei seems to like the idea of them being sliceable, so I don't know how easy it will be to change that.

By the way, it should be isDroppable, not isDropable if you want correct English. The o is soft, not hard, so it's two p's, when adding the able.

@monarchdodra
Collaborator

Based on your description, I don't see how it solves any problems for slicing infinite ranges.

Well, as you said, it would solve the problem by saying that an infinite range simply cannot be slice-able. That's my major point.

My proposal is to have a functionality which can operate in a sensible way on infinite ranges (and normal ranges), without subverting the definition of hasSlicing.

But even if we take that tact, it doesn't really conflict with my changes.

Well, I'd argue you are now inserting formal support for slicing infinite ranges, when the support was just implicit. Your changes that make it that anything not back-assignable would not be slice-able, except for infinite ranges. I say f'em, and break them along with all the other ranges you will be breaking.

However, as much as I really like the idea of making infinite ranges non-sliceable, Andrei seems to like the idea of them being sliceable, so I don't know how easy it will be to change that.

isDroppable and his brother extractSlice would be able to slice infinite ranges just as well as normal finite ranges, without any back-assign pitfalls.

@jmdavis
Collaborator

Well, I'd argue you are now inserting formal support for slicing infinite ranges, when the support was just implicit.

No. It's officially supported right now. It's just that hasSlicing puts no restrictions on the return type (just like no restrictions are currently put on the return type when slicing a finite range).

isDroppable and his brother extractSlice would be able to slice infinite ranges just as well as normal finite ranges, without any back-assign pitfalls.

As I said, that's a separate issue. As long as slicing infinite ranges is legal (as it currently is), my changes are relevant. They only become irrelevant if make slicing infinite ranges illegal, which I suspect that you're going to have a hard time talking Andrei into. You're going to need a very solid proposal, especially when it sounds like what you're proposing complicates things a fair bit. It may be worth doing, but you probably have an uphill battle in convincing Andrei of that. And as long as Andrei hasn't been convinced that infinite ranges shouldn't be sliceable (whether your proposal is accepted or not), then these changes are quite relevant.

Regardless, I'd suggest that you organize your proposal and bring it up for discussion in the newsgroup. Until you've made a solid case for it and convinced Andrei of it, it's going nowhere no matter how good it may be.

@monarchdodra
Collaborator

As long as slicing infinite ranges is legal (as it currently is)

You are right. Never mind then and thanks for the replies. I'll formalize my proposal and present it to the newsgroup.

@jmdavis
Collaborator

The only argument I see for not making the changes to infinite ranges here is to just not require that they use take for slicing until we've sorted out your proposal, but they're going to be sliceable regardless, and we need to move forward with fixing the slicing situation, since it's a known issue with ranges, and we need to sort all of those out sooner rather than later in order to reduce code breakage.

@monarchdodra
Collaborator

Yes, I think you are right. I'll make this my P0, and bring it up after you pull goes through.

@alexrp
Collaborator

Is this good to merge or are there unresolved concerns?

@jmdavis
Collaborator

Is this good to merge or are there unresolved concerns?

Given the nature of the change, I think that Andrei really needs to approve it. Based on previous discussions with him, I think that he's okay with it (at minimum with the changes to finite ranges - I don't know about the infinite ones), but this is the sort of thing that he should probably look at (since it's a fundamental change to one of the major range primitives).

@andralex andralex was assigned
@alexrp
Collaborator

Assigning @andralex.

@monarchdodra
Collaborator

Could I get an opinion on the related :
https://github.com/D-Programming-Language/phobos/blob/a4856a70cc332ce177a6477cf70b5220830d885c/std/range.d#L3999

This is zip's opSlice implementation, in which in which each sub-slice is emplaced into the new zip. Technically, has slicing only require the new slice be assignable to the original slice, but not that the original be constructable from the slice.

I'm hitting the issue in this pull request: #837 , which [improves/fixes] emplace to require strict construction. In my specific pull, the problem is actually bug related, but it does raise the question: Do you think that line of code in Phobos is valid?

@andralex
Owner

I think we should simplify. Sliceable means ctor AND assignment, no ifs and buts. If some of such are not required, fine, just that such rare ranges can't claim to be sliceable.

@jmdavis
Collaborator

I think we should simplify. Sliceable means ctor AND assignment, no ifs and buts. If some of such are not required, fine, just that such rare ranges can't claim to be sliceable.

So, you're saying that both of these lines should compile?

R range;
R r = range[0 .. 1];
r = range[0 .. 1];

the problem the second one is that you then can't slice a const range to a tail-const one, because you can never reassign to const, though I suppose that you could end up with a weird range which is non-const but then isn't assignable from its slice even though it's constructable with it. An alternative would be to simply check that the slice is implicitly convertible to the original, then at least in theory it should work with both construction and assignment, though there may be a weird way to make it so that it doesn't.

But if you're suggesting that we ignore the possibility of slicing a const range, I think that that's a mistake. const already interacts extremely poorly with ranges as it is, and the ability to get a tail-const slice from a const range could be invaluable in interacting appropriately with const ranges. Then again, what's particularly valuable there is being able to slice the entire range, which is a different overload of opSlice which hasSlicing doesn't check for, so maybe not being able to slice a range of elements from a const range really doesn't matter much.

Perhaps we should make it so that hasSlicing will be false for const ranges (with it checking explicitly for both construction and assignment) but add a separate trait for checking for the overload of opSlice which slices the whole range. That could then be used on const ranges if they define it, and it would also be true for container types which define it (as they usually should).

@monarchdodra
Collaborator

Technically, has slicing only require the new slice be assignable to the original slice, but not that the original be constructable from the slice.

Looks like I had understood it backwards actually. Construction back to the original type should be fine, for all the arguments you put forward.

std/algorithm.d
@@ -920,7 +920,7 @@ unittest
InputRange range;
fill(range,filler);
foreach(value;range.arr)
- assert(value == filler);
+ assert(value == filler);
@monarchdodra Collaborator

Nitpick: The assert was inside a foreach, so the indentation was actually correct... Unless I missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmdavis
Collaborator

Okay. It now tests for both construction and assignment from the slice. A potentially major side effect of this is that const and immutable arrays will no longer be considered sliceable (though their slices will be sliceable, since they're tail-const).

We should probably add an eponymous template which tests for whether a type can be fully sliced (that is you can slice it without indices - range[]) without caring whether the slice is assignable to the original (so stuff like containers would be sliceable according to such a trait), but I haven't a clue what to call it (hasFullSlicing?), and it can be added in a separate pull request anyway.

The one thing that I'm still a bit divided on here is slicing infinite ranges. It would be kind of nice to get rid of that in favor of something else given that they really don't follow the normal slicing rules, and if we do replace it with something else (e.g. monarchdodra's isDroppable proposal that he has yet to elaborate on in the newsgroup), then temporarily adding the restrictions on the result of slicing an infinite range may be undesirable. But if we keep the slicing of infinite ranges, then I think that the extra restrictions on slicing infinite ranges that this pull request adds are very desirable.

@monarchdodra
Collaborator

I was kind of waiting for this to pull to go through, but it is ready. I ask the group today then.

@jmdavis
Collaborator

I was kind of waiting for this to pull to go through, but it is ready. I ask the group today then.

I don't really see any reason to delay discussing your proposal until this pull is merged. The issues are related, but I don't see how what you're proposing would be dependent on this pull request.

@jmdavis
Collaborator

Okay, based on the newsgroup discussion on isDroppable, I've now added some requirements to hasSlicing with regards to opDollar. Ideally, opDollar would be outright required for any range with slicing, but unless/until it's changed so that length automatically aliases to opDollar when opDollar isn't defined (issue# 7177), that's probably not a reasonable requirement to make. However, I think that we can make requirements about how opDollar works if it's defined. So, I added

static if(is(typeof(r[0 .. $])))
{
    R t = r[0 .. $];
    t = r[0 .. $];

    static if(!isInfinite!R)
    {
        R u = r[0 .. $ - 1];
        u = r[0 .. $ - 1];
    }
}

to hasSlicing. So now when slicing ranges with opDollar, the result must be the original, even with infinite ranges, and for finite ranges, subtraction on opDollar needs to work.

Given that infinite ranges don't define length and so issue# 7177 doesn't affect them, I suppose that we could require that they define opDollar, but I think that we might as well hold off on that until issue# 7177 is sorted out (if nothing else, because it would be weird to require opDollar for sliceable infinite ranges but not finite ones). It also gives people more time to implement opDollar before we require it now that it works.

std/range.d
((25 lines not shown))
-R r;
-auto s = r[1 .. 2];
-static assert(isInputRange!(typeof(s)));
+R r = void;
+
+static if(isInfinite!R)
+ typeof(take(r, 1)) s = r[1 .. 2];
+else
+ R s = r[1 .. 2];
+
+s = r[1 .. 2];
+
+static if(is(typeof(r[0 .. $])))
+{
+ R t = r[0 .. $];
+ t = r[0 .. $];
@andralex Owner
andralex added a note

I think we should go with the more restrictive but simpler static assert(is(typeof(r[0 .. $] == R));

@jmdavis Collaborator
jmdavis added a note

You mean that you want to check that the slice is the original type exactly? That's not necessarily true with arrays, but as long as the first part still checks for construction and assignment, then the array can't be const or immutable, in which case checking that the slice using $ is the exact type should work.

@jmdavis Collaborator
jmdavis added a note

Checking for the type of the slice being exactly the same as the original doesn't work. I suspect that it's a compiler bug, but I don't know. In particular, iota fails with hasSlicing if I make that change. Asserting static assert(is(typeof(r[0 .. $]) == R)), static assert(is(typeof(r[0 .. $ - 1]) == R)), or static assert(is(typeof(r[0 .. 2]) == R)) results in

Error: static assert (is(Result == Result)) is false

However, the code currently in the pull request works. So, I'm inclined to just go with what's here for the moment. And if we make it more restrictive later, it probably won't break much code (if any), since the main reason for a slice to be a different type would be is if it's a tail-const version of the original range, and that would presumably only be done if the range itself were const. And what's here tests what we really care about. The slice doesn't really need to be exactly the same type as the original so long as it assigns to the original and the original is constructible from it. It's not the same for arrays, after all.

@monarchdodra Collaborator

It's a bug. It's exactly the one I came asking about above.

http://d.puremagic.com/issues/show_bug.cgi?id=8847

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andralex andralex commented on the diff
std/range.d
((10 lines not shown))
+ R s = r[1 .. 2];
+
+ s = r[1 .. 2];
+
+ static if(is(typeof(r[0 .. $])))
+ {
+ R t = r[0 .. $];
+ t = r[0 .. $];
+
+ static if(!isInfinite!R)
+ {
+ R u = r[0 .. $ - 1];
+ u = r[0 .. $ - 1];
+ }
+ }
+
@andralex Owner
andralex added a note

... in which case this code should be amended too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/range.d
((57 lines not shown))
static assert(!hasSlicing!(A));
static assert( hasSlicing!(B));
static assert( hasSlicing!(C));
+ static assert(!hasSlicing!(D));
+
+ //Test that opSlice can return a different type as long as it can be
+ //assigned to the original and the original can be constructed from it.
+ static struct CheckConv
@andralex Owner
andralex added a note

... and therefore this can be eliminated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@alexrp
Collaborator

So, status on this?

@jmdavis
Collaborator

@andralex Is there any reason not to merge this at this point (assuming that it passes the pull tester on its next run)? The one remaining suggestion/issue that I'm aware of was you suggesting to use the more restrictive static assert(is(typeof(r[0 .. $] == R));, but that doesn't currently work due to a compiler bug, and what's here works just fine. It's restrictive enough to guarantee that the operations that we need to work do work, even if it's not as restrictive as static assert(is(typeof(r[0 .. $] == R));, and if need be, once the compiler bug is fixed, we can make hasSlicing more restrictive at that point. But this fixes a major regression with regards to take in addition to the improvements to hasSlicing itself, and I'd very much like to get it merged if there are no more major issues with it.

I did, however, remove the checks for whether a slice could be a different type from the original and still convert to it, so while this version still technically allows it, it's no longer checked for, and if/when we make hasSlicing more restrictive, the unit tests won't be affected. But it's unlikely that anyone will create a range like that anyway, particularly since you have to work at it.

@monarchdodra
Collaborator

Just want to post this bug entry:
http://d.puremagic.com/issues/show_bug.cgi?id=9071

Someone being bit in the ass by the issue you are fixing. It makes for an interesting "real-world" example of what's out there.

@jmdavis
Collaborator

Okay. I adjusted the comments to hasSlicing to indicate that the slice must be the same type as the original like we want to be able to require and adjusted hasSlicing so that it has the extra requirements, but those portions of hasSlicing are commented out with a note about bug# 8847. As soon as that's fixed, they can be uncommented. In the meantime, if this gets merged in, it will almost require that the slice be the same type as the original, and it's unlikely that there will be many (if any) types which will compile with the new version but not compile when bug# 8847 is fixed and the commented stuff in hasSlicing has been uncommented.

@jmdavis
Collaborator

I would like to point out that this pull does fix a rather nasty regression (issue #8556), so it either needs to be merged before the next release, or a version of it without the changes to hasSlicing itself needs to be merged before the next release. Personally, I think that this should be merged and then another pull request done to make hasSlicing stricter like we want after bug# 8847 has been fixed, though given bug# 8847's severity, it would be nice if it were fixed prior to the next release.

@jmdavis
Collaborator

Okay. I just rebased again, so it should be mergeable now. Hopefully this gets merged before yet another change to std.range gets merged in.

@monarchdodra monarchdodra commented on the diff
std/range.d
((75 lines not shown))
+ }
+
+ s = r[1 .. 2];
+
+ static if(is(typeof(r[0 .. $])))
+ {
+ //static assert(is(typeof(r[0 .. $]) == R));
+ R t = r[0 .. $];
+ t = r[0 .. $];
+
+ static if(!isInfinite!R)
+ {
+ //static assert(is(typeof(r[0 .. $ - 1]) == R));
+ R u = r[0 .. $ - 1];
+ u = r[0 .. $ - 1];
+ }
@monarchdodra Collaborator

Related, this kind of opDollar verification feels like it should also be added to isRandomAccessRange, no?

static if (is(typeof(r[$])))
{
    static assert(is(typeof(r.front) == typeof(r[$])));
    static assert(is(typeof(r.front) == typeof(r[$ - 1])));
}

Thoughts?

It's related, but it's a stand alone change, so I can do it if you don't feel like weighing down this already long an important pull.

@jmdavis Collaborator
jmdavis added a note

Honestly, I'd prefer that this just got merged in soon rather then messing with it further. I think that the main reason that it hasn't is that Andrei has been too busy to look at it again. But whether such a change is included or not wouldn't really affect it much, I would think.

I agree that isRandomAccessRange should be changed as you suggest, but if you do that before this is merged, I'll have to rebase this pull yet again, so I'd kind of prefer that you didn't make such a change. I may just add it to this pull this evening if it doesn't get merged in first (which I assume it won't be), since I don't think that such a restriction is particularly controversial, and it is related to these changes. But we'll see if I have time or not (or maybe I'll get improbably lucky and this will get merged first).

What I'd really like to see is issue# 7177 implemented so that we can just require that $ work, as we could then increase the restrictions of hasSlicing all at once. But that requires convincing a compiler dev to do that, and Kenji didn't seem to think that it was a good idea when it came up in a pull request discussion recently. So, maybe a discussion on that should be brought up in the newsgroup or something.

But there's no reason that I'm aware of to delay this pull request at this point other than Andrei being too busy to look it over again.

@jmdavis Collaborator
jmdavis added a note

Oh, drat. A change was made to std.algorithm, and I'm going to have to rebase this again. I'm getting tired of that.

I'll just add the extra restriction to isRandomAcessRange when I do that (hopefully this evening).

@andralex Owner

Sorry about that. Ping me when done. Thanks!

@jmdavis Collaborator
jmdavis added a note

@andralex Okay. I rebased and added the extra requirement to isRandomAccessRange that r[$] work properly if it compiles with the range (though we can't require that it work for all random-access ranges without enhancement #7177 being implemented, just that it does have the correct type if it does compile).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmdavis
Collaborator

@andralex ping?

std/range.d
@@ -7866,8 +8017,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
/++ Ditto +/
- static if(isBidirectionalRange!R)
- void popBack()
+ static if(isBidirectionalRange!R) @property void popBack()
@9rnsr Collaborator
9rnsr added a note

Incorrect @property attribute addition to popBack function.

@jmdavis Collaborator
jmdavis added a note

A botched rebase apparently, as I wasn't doing anything to popBack on anything. I'll fix it shortly.

@jmdavis Collaborator
jmdavis added a note

Okay. It should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/range.d
@@ -7903,8 +8053,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isForwardRange!R) are
$(D true).
+/
- static if(hasMobileElements!R && isForwardRange!R)
- auto moveFront()
+ static if(hasMobileElements!R && isForwardRange!R) @property auto moveFront()
@9rnsr Collaborator
9rnsr added a note

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/range.d
@@ -7914,8 +8063,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isBidirectionalRange!R)
are $(D true).
+/
- static if(hasMobileElements!R && isBidirectionalRange!R)
- auto moveBack()
+ static if(hasMobileElements!R && isBidirectionalRange!R) @property auto moveBack()
@9rnsr Collaborator
9rnsr added a note

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
std/range.d
@@ -7925,8 +8073,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isRandomAccessRange!R)
are $(D true).
+/
- static if(hasMobileElements!R && isRandomAccessRange!R)
- auto moveAt(IndexType)(IndexType index)
+ static if(hasMobileElements!R && isRandomAccessRange!R) @property auto moveAt(IndexType)(IndexType index)
@9rnsr Collaborator
9rnsr added a note

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
jmdavis added some commits
@jmdavis jmdavis Revert "Fix `std.range.takeExactly` trait to reject slices without le…
…ngth."

This reverts commit 6a0b6c4.
f8f2a81
@jmdavis jmdavis Adjustments to take and takeExactly.
takeExactly now returns the same type as take where possible, and
neither take nor takeExactly check hasSlicing for infinite ranges (since
infinite ranges will soon be required to use them for slicing, and that
creates a circular dependency among those 3 templates
24a696a
@jmdavis jmdavis Some whitespace cleanup. 7d5d9fb
@jmdavis jmdavis Adustments to hasSlicing.
Now, it enforces that opSlice returns a range which can be assigned to
the original range type (so that it can be assigned to the original
range as long as the original range isn't const) as long as it's finite,
and it enforces that opSlice's result is the result of take when the
range is infinite.
0d9b31b
@jmdavis jmdavis Fix to Map caused by changes to hasSlicing.
import std.algorithm;
import std.range;

void main()
{
    auto N2 = sequence!"n"(cast(size_t)1).map!"a";
}

ceased to compile, because Map's opSlice won't work anymore over
infinite ranges, because the result can't be reassigned to the original.
57ddea7
@jmdavis jmdavis Add some requirements for opDollar to hasSlicing.
Ideally, opDollar would be outright required for any range with slicing,
but unless/until it's changed so that length automatically aliases to
opDollar when opDollar isn't defined (issue# 7177), that's probably not
a reasonable requirement to make.
a0b82a5
@jmdavis jmdavis Further strengthen hasSlicing.
The extra requirements are not currently enabled because of bug# 8847,
but they're now there, and they're listed as their in the documentation
so that no one will think that they're not supposed to apply.
6022082
@jmdavis jmdavis Strengthened isRandomAccess with regards to $.
It now requires that indexing with $ result in the same type as front if
r[$] works, and if that works, and the range isn't infinite, r[$ - 1]
must be the same type as front. Ideally, we'd require that r[$] work
regardless, but without enhancement #7177 being implemented, that would
likely break too much code, as opDollar was only recently fixed and
probably isn't used much.
a916ad8
@jmdavis jmdavis Removed incorrect @property attributes in std.range.
They were removed previously but reintroduced due to a bad rebase, so
I'm removing them again here.
68db081
@9rnsr
Collaborator

Now I have posted a compiler fix D-Programming-Language/dmd#1380 to fix a bug which reported in issue 8556. It correctly breaks current Phobos build, and this pull can fix the forward reference errors around hasSlicing.

But, sorry, I am not familiar with the recent change of Phobos modules. So I'd like to wait @andralex 's reviewing.

@andralex andralex commented on the diff
std/range.d
((21 lines not shown))
----
-R r;
-auto s = r[1 .. 2];
-static assert(isInputRange!(typeof(s)));
+R r = void;
+
+static if(isInfinite!R)
+ typeof(take(r, 1)) s = r[1 .. 2];
@andralex Owner

auto?

@jmdavis Collaborator
jmdavis added a note

The idea is that r[1 .. 2] must be of the type typeof(take(r, 1)) for infinite ranges. So, using auto would defeat the purpose of the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andralex andralex merged commit af12683 into from
@andralex
Owner

merged, thanks

@jmdavis
Collaborator

Thanks!

Now if we could only have issue# 7177 implemented, then we could fully tighten opSlice and isRandomAccessRange with regards to $... :)

@denis-sh

Great!

The only question: where are the breaking change notices in changelog?

@jmdavis
Collaborator

The only question: where are the breaking change notices in changelog?

They'll be put there. I never put changes to the changelog in pull requests, because frequent rebases are then required. I do better at getting notices in there then most of the other Phobos devs. So, you don't need to worry about that. Thanks for the reminder though.

@denis-sh

By the way, I have never knew how to update chanelog (as there are lots of chanelog files). IMHO, some examples/instructions for contributors will be useful.

@monarchdodra
Collaborator

By the way, I have never knew how to update chanelog (as there are lots of chanelog files). IMHO, some examples/instructions for contributors will be useful.

I think the (new) wiki is the perfect place for this.

@monarchdodra monarchdodra commented on the diff
std/range.d
((64 lines not shown))
- static assert(isInputRange!(typeof(s)));
+
+ static if(isInfinite!R)
+ typeof(take(r, 1)) s = r[1 .. 2];
+ else
+ {
+ //@@@BUG@@@ 8847 makes it so that the three commented out lines
+ //cause Phobos to fail to compile - hence why they're commented
+ //out. They should be uncommented once that bug has been fixed.
+ //static assert(is(typeof(r[1 .. 2]) == R));
+ R s = r[1 .. 2];
+ }
+
+ s = r[1 .. 2];
+
+ static if(is(typeof(r[0 .. $])))
@monarchdodra Collaborator

Sorry for resurrecting this, but I figure this is the best place to discuss it:

Shouldn't we make the r[0 .. $] mandatory for an infinite ranges to satisfy hasSlicing?

Rationale is:

  • Infinite ranges with hasSlicing is new, so we shouldn't be breaking code.
  • The compiler will simply never be able to automatically generate opDollar for them.
  • Range adapters that want to implement slicing for infinite ranges must rely on the range providing r[i .. $].

This is because the adaptor cannot return the type Adaptor!(Take!Range), as it has to return the type Take!(Adaptor!Range) to verify has slicing (this has actually turned out to always be a good thing, in my short experience with this).

Once you take this into account, the only way to implement the adaptor's opSlice operation is to "fast forward" the internal range (r = r[i .. $]), and then return this.take(j - i).

either that or remove the static condition of typeof(take(r, 1)) s = r[1 .. 2];. But I think this requirement is both brilliant for template programming, and the requirement actually forces you to use some very clean implementations.


This question is asked in the context of a zip of slice-able infinite ranges.

If the opSlice operation returns a Zip!(staticMap(Ranges, Take)), then zip will fail the hasSlicing condition. It must return a: Take!(Zip!Ranges) (this is a good thing actually: Take of zip should be much more efficient than zip of takes).

Problem: As explained above, to implement such slicing, zip will need to first slice the internal ranges from i to $. Testing if a range has opDollar is already kind of complicated, and heavily complexifies the implementation (did you see my "chunks" improvement?).

In the context of multiple ranges, just writing the condition of static if ("all ranges are infinite and all of them provide slice to end") is close to nightmarish...


Thoughts? We can take it to newsgroup if you feel it requires more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 17, 2012
  1. @jmdavis
  2. @jmdavis

    Adjustments to take and takeExactly.

    jmdavis authored
    takeExactly now returns the same type as take where possible, and
    neither take nor takeExactly check hasSlicing for infinite ranges (since
    infinite ranges will soon be required to use them for slicing, and that
    creates a circular dependency among those 3 templates
  3. @jmdavis

    Some whitespace cleanup.

    jmdavis authored
  4. @jmdavis

    Adustments to hasSlicing.

    jmdavis authored
    Now, it enforces that opSlice returns a range which can be assigned to
    the original range type (so that it can be assigned to the original
    range as long as the original range isn't const) as long as it's finite,
    and it enforces that opSlice's result is the result of take when the
    range is infinite.
  5. @jmdavis

    Fix to Map caused by changes to hasSlicing.

    jmdavis authored
    import std.algorithm;
    import std.range;
    
    void main()
    {
        auto N2 = sequence!"n"(cast(size_t)1).map!"a";
    }
    
    ceased to compile, because Map's opSlice won't work anymore over
    infinite ranges, because the result can't be reassigned to the original.
  6. @jmdavis

    Add some requirements for opDollar to hasSlicing.

    jmdavis authored
    Ideally, opDollar would be outright required for any range with slicing,
    but unless/until it's changed so that length automatically aliases to
    opDollar when opDollar isn't defined (issue# 7177), that's probably not
    a reasonable requirement to make.
  7. @jmdavis

    Further strengthen hasSlicing.

    jmdavis authored
    The extra requirements are not currently enabled because of bug# 8847,
    but they're now there, and they're listed as their in the documentation
    so that no one will think that they're not supposed to apply.
  8. @jmdavis

    Strengthened isRandomAccess with regards to $.

    jmdavis authored
    It now requires that indexing with $ result in the same type as front if
    r[$] works, and if that works, and the range isn't infinite, r[$ - 1]
    must be the same type as front. Ideally, we'd require that r[$] work
    regardless, but without enhancement #7177 being implemented, that would
    likely break too much code, as opDollar was only recently fixed and
    probably isn't used much.
  9. @jmdavis

    Removed incorrect @property attributes in std.range.

    jmdavis authored
    They were removed previously but reintroduced due to a bad rebase, so
    I'm removing them again here.
This page is out of date. Refresh to see the latest.
Showing with 214 additions and 67 deletions.
  1. +4 −4 std/algorithm.d
  2. +210 −63 std/range.d
View
8 std/algorithm.d
@@ -451,7 +451,7 @@ private struct MapResult(alias fun, Range)
alias length opDollar;
}
- static if (hasSlicing!R)
+ static if (!isInfinite!R && hasSlicing!R)
{
static if (is(typeof(_input[ulong.max .. ulong.max])))
private alias ulong opSlice_t;
@@ -920,7 +920,7 @@ unittest
InputRange range;
fill(range, filler);
foreach (value; range.arr)
- assert(value == filler);
+ assert(value == filler);
}
unittest
{
@@ -947,12 +947,12 @@ unittest
{
int[] a = [1, 2, 3];
immutable(int) b = 0;
- static assert(__traits(compiles, a.fill(b)));
+ static assert(__traits(compiles, a.fill(b)));
}
{
double[] a = [1, 2, 3];
immutable(int) b = 0;
- static assert(__traits(compiles, a.fill(b)));
+ static assert(__traits(compiles, a.fill(b)));
}
}
View
273 std/range.d
@@ -840,12 +840,26 @@ infinite. The following code should compile for any random-access
range.
----
-R r;
-static assert(isForwardRange!R); // range is forward
-static assert(isBidirectionalRange!R || isInfinite!R);
- // range is bidirectional or infinite
-auto e = r[1]; // can index
+// range is finite and bidirectional or infinite and forward.
+static assert(isBidirectionalRange!R ||
+ isForwardRange!R && isInfinite!R);
+
+R r = void;
+auto e = r[1]; // can index
static assert(is(typeof(e) == typeof(r.front))); // same type for indexed and front
+static assert(!isNarrowString!R); // narrow strings cannot be indexed as ranges
+static assert(hasLength!R || isInfinite!R); // must have length or be infinite
+
+// $ must work as it does with arrays if opIndex works with $
+static if(is(typeof(r[$])))
+{
+ static assert(is(typeof(r.front) == typeof(r[$])));
+
+ // $ - 1 doesn't make sense with infinite ranges but needs to work
+ // with finite ones.
+ static if(!isInfinite!R)
+ static assert(is(typeof(r.front) == typeof(r[$ - 1])));
+}
----
The semantics of a random-access range (not checkable during
@@ -871,6 +885,14 @@ template isRandomAccessRange(R)
static assert(is(typeof(e) == typeof(r.front)));
static assert(!isNarrowString!R);
static assert(hasLength!R || isInfinite!R);
+
+ static if(is(typeof(r[$])))
+ {
+ static assert(is(typeof(r.front) == typeof(r[$])));
+
+ static if(!isInfinite!R)
+ static assert(is(typeof(r.front) == typeof(r[$ - 1])));
+ }
}));
}
@@ -1211,39 +1233,132 @@ unittest
}
/**
-Returns $(D true) if $(D R) offers a slicing operator with
-integral boundaries, that in turn returns an input range type. The
-following code should compile for $(D hasSlicing) to be $(D true):
+Returns $(D true) if $(D R) offers a slicing operator with integral boundaries
+that returns a forward range type.
+
+For finite ranges, the result of $(D opSlice) must be of the same type as the
+original range type. If the range defines $(D opDollar), then it must support
+subtraction.
+
+For infinite ranges, when $(I not) using $(D opDollar), the result of
+$(D opSlice) must be the result of $(LREF take) or $(LREF takeExactly) on the
+original range (they both return the same type for infinite ranges). However,
+when using $(D opDollar), the result of $(D opSlice) must be that of the
+original range type.
+
+The following code must compile for $(D hasSlicing) to be $(D true):
----
-R r;
-auto s = r[1 .. 2];
-static assert(isInputRange!(typeof(s)));
+R r = void;
+
+static if(isInfinite!R)
+ typeof(take(r, 1)) s = r[1 .. 2];
@andralex Owner

auto?

@jmdavis Collaborator
jmdavis added a note

The idea is that r[1 .. 2] must be of the type typeof(take(r, 1)) for infinite ranges. So, using auto would defeat the purpose of the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+else
+{
+ static assert(is(typeof(r[1 .. 2]) == R));
+ R s = r[1 .. 2];
+}
+
+s = r[1 .. 2];
+
+static if(is(typeof(r[0 .. $])))
+{
+ static assert(is(typeof(r[0 .. $]) == R));
+ R t = r[0 .. $];
+ t = r[0 .. $];
+
+ static if(!isInfinite!R)
+ {
+ static assert(is(typeof(r[0 .. $ - 1]) == R));
+ R u = r[0 .. $ - 1];
+ u = r[0 .. $ - 1];
+ }
+}
+
+static assert(isForwardRange!(typeof(r[1 .. 2])));
+static assert(hasLength!(typeof(r[1 .. 2])));
----
*/
template hasSlicing(R)
{
- enum bool hasSlicing = !isNarrowString!R && is(typeof(
+ enum bool hasSlicing = isForwardRange!R && !isNarrowString!R && is(typeof(
(inout int = 0)
{
R r = void;
- auto s = r[1 .. 2];
- static assert(isInputRange!(typeof(s)));
+
+ static if(isInfinite!R)
+ typeof(take(r, 1)) s = r[1 .. 2];
+ else
+ {
+ //@@@BUG@@@ 8847 makes it so that the three commented out lines
+ //cause Phobos to fail to compile - hence why they're commented
+ //out. They should be uncommented once that bug has been fixed.
+ //static assert(is(typeof(r[1 .. 2]) == R));
+ R s = r[1 .. 2];
+ }
+
+ s = r[1 .. 2];
+
+ static if(is(typeof(r[0 .. $])))
@monarchdodra Collaborator

Sorry for resurrecting this, but I figure this is the best place to discuss it:

Shouldn't we make the r[0 .. $] mandatory for an infinite ranges to satisfy hasSlicing?

Rationale is:

  • Infinite ranges with hasSlicing is new, so we shouldn't be breaking code.
  • The compiler will simply never be able to automatically generate opDollar for them.
  • Range adapters that want to implement slicing for infinite ranges must rely on the range providing r[i .. $].

This is because the adaptor cannot return the type Adaptor!(Take!Range), as it has to return the type Take!(Adaptor!Range) to verify has slicing (this has actually turned out to always be a good thing, in my short experience with this).

Once you take this into account, the only way to implement the adaptor's opSlice operation is to "fast forward" the internal range (r = r[i .. $]), and then return this.take(j - i).

either that or remove the static condition of typeof(take(r, 1)) s = r[1 .. 2];. But I think this requirement is both brilliant for template programming, and the requirement actually forces you to use some very clean implementations.


This question is asked in the context of a zip of slice-able infinite ranges.

If the opSlice operation returns a Zip!(staticMap(Ranges, Take)), then zip will fail the hasSlicing condition. It must return a: Take!(Zip!Ranges) (this is a good thing actually: Take of zip should be much more efficient than zip of takes).

Problem: As explained above, to implement such slicing, zip will need to first slice the internal ranges from i to $. Testing if a range has opDollar is already kind of complicated, and heavily complexifies the implementation (did you see my "chunks" improvement?).

In the context of multiple ranges, just writing the condition of static if ("all ranges are infinite and all of them provide slice to end") is close to nightmarish...


Thoughts? We can take it to newsgroup if you feel it requires more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ //static assert(is(typeof(r[0 .. $]) == R));
+ R t = r[0 .. $];
+ t = r[0 .. $];
+
+ static if(!isInfinite!R)
+ {
+ //static assert(is(typeof(r[0 .. $ - 1]) == R));
+ R u = r[0 .. $ - 1];
+ u = r[0 .. $ - 1];
+ }
@monarchdodra Collaborator

Related, this kind of opDollar verification feels like it should also be added to isRandomAccessRange, no?

static if (is(typeof(r[$])))
{
    static assert(is(typeof(r.front) == typeof(r[$])));
    static assert(is(typeof(r.front) == typeof(r[$ - 1])));
}

Thoughts?

It's related, but it's a stand alone change, so I can do it if you don't feel like weighing down this already long an important pull.

@jmdavis Collaborator
jmdavis added a note

Honestly, I'd prefer that this just got merged in soon rather then messing with it further. I think that the main reason that it hasn't is that Andrei has been too busy to look at it again. But whether such a change is included or not wouldn't really affect it much, I would think.

I agree that isRandomAccessRange should be changed as you suggest, but if you do that before this is merged, I'll have to rebase this pull yet again, so I'd kind of prefer that you didn't make such a change. I may just add it to this pull this evening if it doesn't get merged in first (which I assume it won't be), since I don't think that such a restriction is particularly controversial, and it is related to these changes. But we'll see if I have time or not (or maybe I'll get improbably lucky and this will get merged first).

What I'd really like to see is issue# 7177 implemented so that we can just require that $ work, as we could then increase the restrictions of hasSlicing all at once. But that requires convincing a compiler dev to do that, and Kenji didn't seem to think that it was a good idea when it came up in a pull request discussion recently. So, maybe a discussion on that should be brought up in the newsgroup or something.

But there's no reason that I'm aware of to delay this pull request at this point other than Andrei being too busy to look it over again.

@jmdavis Collaborator
jmdavis added a note

Oh, drat. A change was made to std.algorithm, and I'm going to have to rebase this again. I'm getting tired of that.

I'll just add the extra restriction to isRandomAcessRange when I do that (hopefully this evening).

@andralex Owner

Sorry about that. Ping me when done. Thanks!

@jmdavis Collaborator
jmdavis added a note

@andralex Okay. I rebased and added the extra requirement to isRandomAccessRange that r[$] work properly if it compiles with the range (though we can't require that it work for all random-access ranges without enhancement #7177 being implemented, just that it does have the correct type if it does compile).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
@andralex Owner
andralex added a note

... in which case this code should be amended too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ static assert(isForwardRange!(typeof(r[1 .. 2])));
+ static assert(hasLength!(typeof(r[1 .. 2])));
}));
}
unittest
{
static assert( hasSlicing!(int[]));
+ static assert( hasSlicing!(const(int)[]));
+ static assert(!hasSlicing!(const int[]));
static assert( hasSlicing!(inout(int)[]));
+ static assert(!hasSlicing!(inout int []));
+ static assert( hasSlicing!(immutable(int)[]));
+ static assert(!hasSlicing!(immutable int[]));
static assert(!hasSlicing!string);
-
- struct A { int opSlice(uint, uint); }
- struct B { int[] opSlice(uint, uint); }
- struct C { @disable this(); int[] opSlice(size_t, size_t); }
+ static assert( hasSlicing!dstring);
+
+ enum rangeFuncs = "@property int front();" ~
+ "void popFront();" ~
+ "@property bool empty();" ~
+ "@property auto save() { return this; }" ~
+ "@property size_t length();";
+
+ struct A { mixin(rangeFuncs); int opSlice(size_t, size_t); }
+ struct B { mixin(rangeFuncs); B opSlice(size_t, size_t); }
+ struct C { mixin(rangeFuncs); @disable this(); C opSlice(size_t, size_t); }
+ struct D { mixin(rangeFuncs); int[] opSlice(size_t, size_t); }
static assert(!hasSlicing!(A));
static assert( hasSlicing!(B));
static assert( hasSlicing!(C));
+ static assert(!hasSlicing!(D));
+
+ struct InfOnes
+ {
+ enum empty = false;
+ void popFront() {}
+ @property int front() { return 1; }
+ @property InfOnes save() { return this; }
+ auto opSlice(size_t i, size_t j) { return takeExactly(this, j - i); }
+ auto opSlice(size_t i, Dollar d) { return this; }
+
+ struct Dollar {}
+ Dollar opDollar() const { return Dollar.init; }
+ }
+
+ static assert(hasSlicing!InfOnes);
}
/**
@@ -2512,8 +2627,10 @@ assert(equal(s, [ 1, 2, 3, 4, 5 ][]));
----
*/
struct Take(Range)
-if (isInputRange!(Unqual!Range)
- && !(hasSlicing!(Unqual!Range) || is(Range T == Take!T)))
+if (isInputRange!(Unqual!Range) &&
+ //take _cannot_ test hasSlicing on infinite ranges, because hasSlicing uses
+ //take for slicing infinite ranges.
+ !((!isInfinite!(Unqual!Range) && hasSlicing!(Unqual!Range)) || is(Range T == Take!T)))
{
private alias Unqual!Range R;
@@ -2668,32 +2785,24 @@ if (isInputRange!(Unqual!Range)
// This template simply aliases itself to R and is useful for consistency in
// generic code.
template Take(R)
-if (isInputRange!(Unqual!R) && (hasSlicing!(Unqual!R) || is(R T == Take!T)))
+if (isInputRange!(Unqual!R) &&
+ ((!isInfinite!(Unqual!R) && hasSlicing!(Unqual!R)) || is(R T == Take!T)))
{
alias R Take;
}
-// take for ranges with slicing (finite or infinite)
+// take for finite ranges with slicing
/// ditto
Take!R take(R)(R input, size_t n)
-if (isInputRange!(Unqual!R) && hasSlicing!(Unqual!R))
+if (isInputRange!(Unqual!R) && !isInfinite!(Unqual!R) && hasSlicing!(Unqual!R))
{
- static if (hasLength!R)
- {
- // @@@BUG@@@
- //return input[0 .. min(n, $)];
- return input[0 .. min(n, input.length)];
- }
- else
- {
- static assert(isInfinite!R,
- "Nonsensical finite range with slicing but no length");
- return input[0 .. n];
- }
+ // @@@BUG@@@
+ //return input[0 .. min(n, $)];
+ return input[0 .. min(n, input.length)];
}
// take(take(r, n1), n2)
-Take!(R) take(R)(R input, size_t n)
+Take!R take(R)(R input, size_t n)
if (is(R T == Take!T))
{
return R(input.source, min(n, input._maxAvailable));
@@ -2701,7 +2810,7 @@ if (is(R T == Take!T))
// Regular take for input ranges
Take!(R) take(R)(R input, size_t n)
-if (isInputRange!(Unqual!R) && !hasSlicing!(Unqual!R) && !is(R T == Take!T))
+if (isInputRange!(Unqual!R) && (isInfinite!(Unqual!R) || !hasSlicing!(Unqual!R) && !is(R T == Take!T)))
{
return Take!R(input, n);
}
@@ -2731,7 +2840,6 @@ unittest
takeMyStrAgain = take(takeMyStr, 10);
assert(equal(takeMyStrAgain, "This is"));
-
foreach(DummyType; AllDummyRanges) {
DummyType dummy;
auto t = take(dummy, 5);
@@ -2756,6 +2864,9 @@ unittest
// also have random access.
assert(equal(t, [1,2,3,4,5]));
+
+ //Test that take doesn't wrap the result of take.
+ assert(take(t, 4) == take(dummy, 4));
}
immutable myRepeat = repeat(1);
@@ -2785,23 +2896,30 @@ n) elements. Consequently, the result of $(D takeExactly(range, n))
always defines the $(D length) property (and initializes it to $(D n))
even when $(D range) itself does not define $(D length).
-If $(D R) has slicing and its slice has length, $(D takeExactly) simply
-returns a slice of $(D range).
-Otherwise if $(D R) is an input range, the type of the result
-is an input range with length. Finally, if $(D R) is a forward range
-(including bidirectional), the type of the result is a forward range
-with length.
+The result of $(D takeExactly) is identical to that of $(LREF take) in
+cases where the original range defines $(D length) or is infinite.
*/
auto takeExactly(R)(R range, size_t n)
-if (isInputRange!R && !hasSlicing!R)
+if (isInputRange!R)
{
static if (is(typeof(takeExactly(range._input, n)) == R))
{
+ assert(n <= range._n,
+ "Attempted to take more than the length of the range with takeExactly.");
// takeExactly(takeExactly(r, n1), n2) has the same type as
// takeExactly(r, n1) and simply returns takeExactly(r, n2)
range._n = n;
return range;
}
+ //Also covers hasSlicing!R for finite ranges.
+ else static if (hasLength!R)
+ {
+ assert(n <= range.length,
+ "Attempted to take more than the length of the range with takeExactly.");
+ return take(range, n);
+ }
+ else static if (isInfinite!R)
+ return Take!R(range, n);
else
{
static struct Result
@@ -2830,12 +2948,6 @@ if (isInputRange!R && !hasSlicing!R)
}
}
-auto takeExactly(R)(R range, size_t n)
-if (hasSlicing!R && hasLength!(typeof(range[0 .. n])))
-{
- return range[0 .. n];
-}
-
unittest
{
auto a = [ 1, 2, 3, 4, 5 ];
@@ -2856,8 +2968,42 @@ unittest
assert(e.length == 3);
assert(e.front == 1);
- assert(equal(takeExactly(e, 4), [1, 2, 3, 4]));
- // b[1]++;
+ assert(equal(takeExactly(e, 3), [1, 2, 3]));
+
+ //Test that take and takeExactly are the same for ranges which define length
+ //but aren't sliceable.
+ struct L
+ {
+ @property auto front() { return _arr[0]; }
+ @property bool empty() { return _arr.empty; }
+ void popFront() { _arr.popFront(); }
+ @property size_t length() { return _arr.length; }
+ int[] _arr;
+ }
+ static assert(is(typeof(take(L(a), 3)) == typeof(takeExactly(L(a), 3))));
+ assert(take(L(a), 3) == takeExactly(L(a), 3));
+
+ //Test that take and takeExactly are the same for ranges which are sliceable.
+ static assert(is(typeof(take(a, 3)) == typeof(takeExactly(a, 3))));
+ assert(take(a, 3) == takeExactly(a, 3));
+
+ //Test that take and takeExactly are the same for infinite ranges.
+ auto inf = repeat(1);
+ static assert(is(typeof(take(inf, 5)) == Take!(typeof(inf))));
+ assert(take(inf, 5) == takeExactly(inf, 5));
+
+ //Test that take and takeExactly are _not_ the same for ranges which don't
+ //define length.
+ static assert(!is(typeof(take(filter!"true"(a), 3)) == typeof(takeExactly(filter!"true"(a), 3))));
+
+ foreach(DummyType; AllDummyRanges)
+ {
+ DummyType dummy;
+ auto t = takeExactly(dummy, 5);
+
+ //Test that takeExactly doesn't wrap the result of takeExactly.
+ assert(takeExactly(t, 4) == takeExactly(dummy, 4));
+ }
}
/**
@@ -3038,7 +3184,9 @@ unittest
@disable this();
this(int[] arr) { _arr = arr; }
mixin(genInput());
+ @property auto save() { return this; }
auto opSlice(size_t i, size_t j) { return typeof(this)(_arr[i .. j]); }
+ @property size_t length() { return _arr.length; }
int[] _arr;
}
@@ -3068,7 +3216,9 @@ unittest
{
this(int[] arr) { _arr = arr; }
mixin(genInput());
+ @property auto save() { return new typeof(this)(_arr); }
auto opSlice(size_t i, size_t j) { return new typeof(this)(_arr[i .. j]); }
+ @property size_t length() { return _arr.length; }
int[] _arr;
}
@@ -4855,6 +5005,7 @@ unittest
unittest
{
auto odds = sequence!("a[0] + n * a[1]")(1, 2);
+ static assert(hasSlicing!(typeof(odds)));
// static slicing tests
assert(equal(odds[0 .. 5], take(odds, 5)));
@@ -6728,11 +6879,11 @@ template InputRangeObject(R) if (isInputRange!(Unqual!R)) {
static if (isBidirectionalRange!R) {
@property E back() { return _range.back; }
- @property E moveBack() {
+ E moveBack() {
return .moveBack(_range);
}
- @property void popBack() { return _range.popBack(); }
+ void popBack() { return _range.popBack(); }
static if (hasAssignableElements!R) {
@property void back(E newVal) {
@@ -7866,8 +8017,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
/++ Ditto +/
- static if(isBidirectionalRange!R)
- void popBack()
+ static if(isBidirectionalRange!R) void popBack()
{
return (*_range).popBack();
}
@@ -7903,8 +8053,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isForwardRange!R) are
$(D true).
+/
- static if(hasMobileElements!R && isForwardRange!R)
- auto moveFront()
+ static if(hasMobileElements!R && isForwardRange!R) auto moveFront()
{
return (*_range).moveFront();
}
@@ -7914,8 +8063,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isBidirectionalRange!R)
are $(D true).
+/
- static if(hasMobileElements!R && isBidirectionalRange!R)
- auto moveBack()
+ static if(hasMobileElements!R && isBidirectionalRange!R) auto moveBack()
{
return (*_range).moveBack();
}
@@ -7925,8 +8073,7 @@ assert(buffer2 == [11, 12, 13, 14, 15]);
Only defined if $(D hasMobileElements!R) and $(D isRandomAccessRange!R)
are $(D true).
+/
- static if(hasMobileElements!R && isRandomAccessRange!R)
- auto moveAt(IndexType)(IndexType index)
+ static if(hasMobileElements!R && isRandomAccessRange!R) auto moveAt(IndexType)(IndexType index)
if(is(typeof((*_range).moveAt(index))))
{
return (*_range).moveAt(index);
Something went wrong with that request. Please try again.