Bug 8556 – Using take results in a corrupted call to opSlice
Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2012-08-17T02:41:00Z
Last change time
2012-12-28T17:10:18Z
Keywords
pull, wrong-code
Assigned to
nobody
Creator
issues.dlang
Comments
Comment #0 by issues.dlang — 2012-08-17T02:41:15Z
Okay. This program
import std.algorithm;
import std.range;
import std.stdio;
import std.string;
import std.traits;
struct Cycle(Range)
if (isForwardRange!(Unqual!Range) && !isInfinite!(Unqual!Range))
{
alias Unqual!Range R;
R _original;
size_t _index;
this(R input, size_t index = 0) { _original = input; _index = index; }
@property auto ref front()
{
return _original[_index % _original.length];
}
enum bool empty = false;
void popFront() { ++_index; }
auto opSlice(size_t i, size_t j)
{
assert(j >= i, format("%s %s %s", i, j, _index));
writefln("%s %s %s", i, j, _index);
return takeExactly(typeof(this)(_original.save, _index + i), j - i);
}
}
Cycle!R cycle(R)(R input, size_t index = 0)
if (isRandomAccessRange!(Unqual!R) && !isInfinite!(Unqual!R))
{
return Cycle!R(input, index);
}
void main()
{
uint[] arr = [1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U, 9U, 10U];
auto t = take(cycle(arr), 20);
auto cx = cycle(arr);
auto slice = cx[23 .. 33];
assert(equal(slice, [4, 5, 6, 7, 8, 9, 10, 1, 2, 3]), format("%s", array(slice)));
}
seems to give different results depending on the architecture, and small tweaks to it seem to result in different results fairly easily. Given the code above, it should print out
23 33 0
and pass the assertions, but right now it prints out
[email protected](28): 140464871555008 10 0
and the exact number for the first index varies between runs of the program. If I were to change opSlice to take ints, then I get
0 10 0
[email protected](48): [1,2,3,4,5,6,7,8,9,10]
and the results actually seem to be consistent.
If I remove the call to take, then the code works just fine. So, it looks like for some reason, that call to take is resulting in the indices to opSlice being corrupted, and rather than 22 and 33, it's more or less random, depending on what other code is there (code which should have _zero_ effect on the indices) and potentially changing between runs of the program.
Interestingly enough, if I compile with -O, I get this error in opSlice:
q.d(28): Error: variable j used before set
q.d(28): Error: variable i used before set
and if I compile with -inline, I get
q.d(28): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(28): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(28): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(28): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(29): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(29): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(30): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(30): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
q.d(30): Error: function q.Cycle!(uint[]).Cycle.opSlice is a nested function and cannot be accessed from D main
dmd: glue.c:735: virtual void FuncDeclaration::toObjFile(int): Assertion `!vthis->csym' failed.
Aborted
So, maybe there's something wrong with the lowering code? take does call the slice operator, but using the slice operator twice rather than calling take and then using it seems to work just fine. All in all, this looks really weird.
Comment #1 by issues.dlang — 2012-08-19T03:01:19Z
Thus far, it seems that this bug makes it impossible to add opSlice to std.range.Cycle.
Comment #2 by issues.dlang — 2012-10-09T19:22:06Z
*** Issue 8790 has been marked as a duplicate of this issue. ***
Comment #3 by issues.dlang — 2012-10-09T19:23:44Z
I'm changing this to a regression, because bug# 8790 shows an example of where this causes a regression:
import std.algorithm;
import std.range;
void main() {
auto N2 = sequence!"n"(cast(size_t)1).map!"a";
}
Comment #4 by issues.dlang — 2012-10-09T20:52:16Z
Okay. Here's a test case which does not rely on either take, takeExactly or hasSlicing (since they will likely be changed soon so that they don't have a circular dependency, fixing the immediate problem in Phobos but not fixing the compiler bug):
import std.algorithm;
import std.range;
import std.stdio;
import std.string;
import std.traits;
template isSliceable(R)
{
enum bool isSliceable = !isNarrowString!R && is(typeof(
(inout int _dummy=0)
{
R r = void;
auto s = r[1 .. 2];
static assert(isInputRange!(typeof(s)));
}));
}
struct Grab(Range)
if (isInputRange!(Unqual!Range)
&& !(isSliceable!(Unqual!Range) || is(Range T == Grab!T)))
{
private alias Unqual!Range R;
// User accessible in read and write
public R source;
private size_t _maxAvailable;
private enum bool byRef = is(typeof(&_input.front) == ElementType!(R)*);
alias R Source;
@property bool empty()
{
return _maxAvailable == 0 || source.empty;
}
@property auto ref front()
{
assert(!empty,
"Attempting to fetch the front of an empty "
~ Grab.stringof);
return source.front;
}
void popFront()
{
assert(!empty,
"Attempting to popFront() past the end of a "
~ Grab.stringof);
source.popFront();
--_maxAvailable;
}
static if (isForwardRange!R)
@property Grab save()
{
return Grab(source.save, _maxAvailable);
}
static if (isInfinite!R)
{
@property size_t length() const
{
return _maxAvailable;
}
alias length opDollar;
}
else static if (hasLength!R)
{
@property size_t length()
{
return min(_maxAvailable, source.length);
}
alias length opDollar;
}
}
Grab!R grab(R)(R input, size_t n)
if (isInputRange!(Unqual!R) && isSliceable!(Unqual!R))
{
static if (hasLength!R)
return input[0 .. min(n, input.length)];
else
return input[0 .. n];
}
Grab!(R) grab(R)(R input, size_t n)
if (is(R T == Grab!T))
{
return R(input.source, min(n, input._maxAvailable));
}
Grab!R grab(R)(R input, size_t n)
if (isInputRange!(Unqual!R) && !isSliceable!(Unqual!R) && !is(R T == Grab!T))
{
return Grab!R(input, n);
}
auto grabExactly(R)(R range, size_t n)
if (isInputRange!R && !isSliceable!R)
{
static if (is(typeof(grabExactly(range._input, n)) == R))
{
range._n = n;
return range;
}
else
{
static struct Result
{
R _input;
private size_t _n;
@property bool empty() const { return !_n; }
@property auto ref front()
{
assert(_n > 0, "front() on an empty " ~ Result.stringof);
return _input.front();
}
void popFront() { _input.popFront(); --_n; }
@property size_t length() const { return _n; }
alias length opDollar;
static if (isForwardRange!R)
@property auto save() { return this; }
}
return Result(range, n);
}
}
auto grabExactly(R)(R range, size_t n)
if (isSliceable!R)
{
return range[0 .. n];
}
struct Circle(Range)
if (isForwardRange!(Unqual!Range) && !isInfinite!(Unqual!Range))
{
alias Unqual!Range R;
R _original;
size_t _index;
this(R input, size_t index = 0) { _original = input; _index = index; }
@property auto ref front()
{
return _original[_index % _original.length];
}
enum bool empty = false;
void popFront() { ++_index; }
auto opSlice(size_t i, size_t j)
{
assert(j >= i, format("%s %s %s", i, j, _index));
writefln("%s %s %s", i, j, _index);
return grabExactly(typeof(this)(_original.save, _index + i), j - i);
}
}
Circle!R circle(R)(R input, size_t index = 0)
if (isRandomAccessRange!(Unqual!R) && !isInfinite!(Unqual!R))
{
return Circle!R(input, index);
}
void main()
{
uint[] arr = [1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U, 9U, 10U];
auto t = grab(circle(arr), 20);
auto cx = circle(arr);
auto slice = cx[23 .. 33];
assert(equal(slice, [4, 5, 6, 7, 8, 9, 10, 1, 2, 3]), format("%s", array(slice)));
}
It can probably be reduced further, but that already reduces it a fair bit. It's been copied from Phobos with the names being tweaked (hasSlicing -> isSliceable, take -> grab, takeExactly -> grabExactly) and some of the unnecessary parts stripped out. So, this should provide a reproducible test case even once hasSlicing and take have been fixed. I believe that the key thing is the circular dependencies involved.
When will this pull request be merged? One of my Phobos pull requests is affected by this bug. :-(
Comment #7 by issues.dlang — 2012-10-10T21:29:44Z
The pull request is less than 24 hours old, and Phobos pull requests are frequently around for weeks before they get merged in. We've been doing better about processing them in a timely manner, but don't expect it to take just a day or two, especially if anything about it is considered controversial as this pull request potentially is. There's a decent chance that it'll be less than a week, but who knows.