Skip to content
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.

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

Fix issue 21129: Make OnlyResult store a value tuple instead of a static array of CommonType. #7584

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 58 additions & 26 deletions std/range/package.d
Expand Up @@ -9744,11 +9744,14 @@ public:
assert([1].map!(x => x).slide(2).equal!equal([[1]]));
}

private struct OnlyResult(T, size_t arity)
private struct OnlyResult(Values...)
if (Values.length > 1)
{
private this(Values...)(return scope auto ref Values values)
private enum arity = Values.length;

private this(return scope ref Values values)
{
this.data = [values];
this.values = values;
this.backIndex = arity;
}

Expand All @@ -9757,10 +9760,10 @@ private struct OnlyResult(T, size_t arity)
return frontIndex >= backIndex;
}

T front() @property
CommonType!Values front() @property
{
assert(!empty, "Attempting to fetch the front of an empty Only range");
return data[frontIndex];
return this[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an error?

Copy link
Member

Choose a reason for hiding this comment

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

0 means frontIndex in this context, see opIndex implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind

}

void popFront()
Expand All @@ -9769,10 +9772,10 @@ private struct OnlyResult(T, size_t arity)
++frontIndex;
}

T back() @property
CommonType!Values back() @property
{
assert(!empty, "Attempting to fetch the back of an empty Only range");
return data[backIndex - 1];
return this[$ - 1];
}

void popBack()
Expand All @@ -9793,12 +9796,15 @@ private struct OnlyResult(T, size_t arity)

alias opDollar = length;

T opIndex(size_t idx)
CommonType!Values opIndex(size_t idx)
{
// when i + idx points to elements popped
// with popBack
assert(idx < length, "Attempting to fetch an out of bounds index from an Only range");
return data[frontIndex + idx];
final switch (frontIndex + idx)
static foreach (i, T; Values)
case i:
return values[i];
}

OnlyResult opSlice()
Expand Down Expand Up @@ -9830,16 +9836,16 @@ private struct OnlyResult(T, size_t arity)
{
import std.traits : hasElaborateAssign;
static if (hasElaborateAssign!T)
private T[arity] data;
private Values values;
else
private T[arity] data = void;
private Values values = void;
}
else
private T[arity] data;
private Values values;
}

// Specialize for single-element results
private struct OnlyResult(T, size_t arity : 1)
private struct OnlyResult(T)
{
@property T front()
{
Expand Down Expand Up @@ -9903,7 +9909,7 @@ private struct OnlyResult(T, size_t arity : 1)
}

// Specialize for the empty range
private struct OnlyResult(T, size_t arity : 0)
private struct OnlyResult()
{
private static struct EmptyElementType {}

Expand Down Expand Up @@ -9953,7 +9959,7 @@ See_Also: $(LREF chain) to chain ranges
auto only(Values...)(return scope Values values)
if (!is(CommonType!Values == void) || Values.length == 0)
{
return OnlyResult!(CommonType!Values, Values.length)(values);
return OnlyResult!Values(values);
}

///
Expand All @@ -9977,17 +9983,6 @@ if (!is(CommonType!Values == void) || Values.length == 0)
.equal("T.D.P.L"));
}

@safe unittest
{
// Verify that the same common type and same arity
// results in the same template instantiation
static assert(is(typeof(only(byte.init, int.init)) ==
typeof(only(int.init, byte.init))));

static assert(is(typeof(only((const(char)[]).init, string.init)) ==
typeof(only((const(char)[]).init, (const(char)[]).init))));
}

// https://issues.dlang.org/show_bug.cgi?id=20314
@safe unittest
{
Expand Down Expand Up @@ -10161,6 +10156,43 @@ if (!is(CommonType!Values == void) || Values.length == 0)
cast(void) only(test, test); // Works with mutable indirection
}

// https://issues.dlang.org/show_bug.cgi?id=21129
@safe unittest
{
auto range = () @safe {
const(char)[5] staticStr = "Hello";

// `only` must store a char[5] - not a char[]!
return only(staticStr, " World");
} ();

assert(range.join == "Hello World");
}

// https://issues.dlang.org/show_bug.cgi?id=21129
@safe unittest
{
struct AliasedString
{
const(char)[5] staticStr = "Hello";

@property const(char)[] slice() const
{
return staticStr[];
}
alias slice this;
}

auto range = () @safe {
auto hello = AliasedString();

// a copy of AliasedString is stored in the range.
return only(hello, " World");
} ();

assert(range.join == "Hello World");
}

/**
Iterate over `range` with an attached index variable.

Expand Down