tweaks in fill(val), fill(range) and copy() #802

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Collaborator

monarchdodra commented Sep 18, 2012

Originally meant as a couple of improvements in copy, fill(val) and fill(range):
*Removing case in fill(range) for infinite ranges with slicing. Also removing lots of un-needed stuff when forwarding to copy, as copy is only optimized in case of arrays.
*Re-writing fill(val) to use opSliceAssign, instead of relying on mem-copies. This makes it more simple and more efficient
*Added a case for optimized array copy even with overlap (copy by bands of non-overlap).

While I was on the subject, I've formalized the effect of these methods on reference semantics range: As a general rule:
"source" is always advanced
"target" is not advanced, if possible (target is RA with length). Other wise, it is advanced.
These are TENTATIVE changes, and I think should be discussed.

This does NOT change any document existing behavior.

This change rational is in this post:
http://forum.dlang.org/thread/oiczxzkzxketxitncghl@forum.dlang.org

Collaborator

monarchdodra commented Sep 18, 2012

bit of compile errors, closing temporarilly

Collaborator

monarchdodra commented Sep 18, 2012

Fixed, whew!

@monarchdodra monarchdodra reopened this Sep 18, 2012

{
alias ElementType!Range T;
- static if (hasElaborateCopyConstructor!T || !isDynamicArray!Range)
+
+ static if (isArray!Range && is(T == Unqual!Value))
@andralex

andralex Sep 28, 2012

Owner

static if (is(typeof(range[] = filler)))

+ //Raw array, opSliceAssign will work just fine.
+ range[] = filler;
+ }
+ else static if (isArray!Range && isAssignable!(T, T))
@andralex

andralex Sep 28, 2012

Owner

else static if (is(typeof(range[1 .. $] = range.front)))

+ {
+ //This case is to avoid emptying range
+ auto len = range.length;
+ for( typeof(len) i ; i < len ; ++i )
@andralex

andralex Sep 28, 2012

Owner

foreach (i; 0 .. range.length)

+
+If $(D range) uses reference semantics and models a $(D RandomAccessRange)
+with $(D hasLength), it will not be modified. Otherwise, $(D fill) will empty
+$(D range) with successive calls to $(D popFront).
*/
@andralex

andralex Sep 28, 2012

Owner

This semantics is a bit convoluted for the sake of a very special case: input ranges that have assignable elements. I wonder whether we should better just support forward ranges with fill. Then we'd use save to copy the range and fill it with the classic approach.

+If $(D range) uses reference semantics and models a $(D RandomAccessRange)
+with $(D hasLength), it will not be modified. Otherwise, $(D fill) with empty
+$(D range) with successive calls to $(D popFront).
+
@andralex

andralex Sep 28, 2012

Owner

same note here

{
- return genericImpl(source, target);
+ auto overlaps = !(source.ptr + source.length <= target.ptr ||
+ target.ptr + target.length <= source.ptr);
@andralex

andralex Sep 28, 2012

Owner

there's a primitive overlap or so

Collaborator

monarchdodra commented Sep 28, 2012

Looking back on this 2 week old code with a cool head, I realize there is a lot of stupid in there. However, it does correct even more stupid I had put in there 2 months ago... Closing temporarilly while I remove the last of the stupid >:(

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