Thread overview | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
February 04, 2016 Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
This code: void main() { import std.typecons; auto tp = tuple!("a", "b", "c")(10, false, "hello"); auto u0 = tp.slice!(0, tp.length); auto u1 = tp.slice!(1, tp.length); auto u2 = tp.slice!(2, tp.length); static assert(is(typeof(u0) == Tuple!(int, "a", bool, "b", string, "c"))); static assert(is(typeof(u1) == Tuple!(bool, "b", string, "c"))); static assert(is(typeof(u2) == Tuple!(string, "c"))); assert(u2.c == "hello"); assert(u0.c == "hello"); assert(u1.c == "hello"); // This assert fails. Why? } core.exception.AssertError@erasetype.d(16): Assertion failure ---------------- 4 erasetype 0x0000000100ce8128 _d_assert + 104 5 erasetype 0x0000000100cd12fe void erasetype.__assert(int) + 38 6 erasetype 0x0000000100cd12aa _Dmain + 250 7 erasetype 0x0000000100cf7297 D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ6runAllMFZ9__lambda1MFZv + 39 8 erasetype 0x0000000100cf71cf void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).tryExec(scope void delegate()) + 55 9 erasetype 0x0000000100cf723c void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).runAll() + 44 10 erasetype 0x0000000100cf71cf void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).tryExec(scope void delegate()) + 55 11 erasetype 0x0000000100cf7121 _d_run_main + 497 12 erasetype 0x0000000100cd133f main + 15 13 libdyld.dylib 0x00007fff8a1345c8 start + 0 14 ??? 0x0000000000000000 0x0 + 0 Why does 'u1' behave differently? I'm thinking it's a bug, but I haven't worked much with tuples, so maybe I'm missing something? Thanks, Saurabh |
February 04, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Saurabh Das | On Thursday, 4 February 2016 at 12:28:39 UTC, Saurabh Das wrote: > This code: > [...] Update: Simplified, this also doesn't work: void main() { import std.typecons; auto tp = tuple(10, false, "hello"); auto u0 = tp.slice!(0, tp.length); auto u1 = tp.slice!(1, tp.length); auto u2 = tp.slice!(2, tp.length); static assert(is(typeof(u0) == Tuple!(int, bool, string))); static assert(is(typeof(u1) == Tuple!(bool, string))); static assert(is(typeof(u2) == Tuple!(string))); assert(u2[0] == "hello"); assert(u0[2] == "hello"); assert(u1[1] == "hello"); // This fails } rdmd erasetype.d core.exception.AssertError@erasetype.d(16): Assertion failure Any ideas? |
February 04, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Saurabh Das | Am Thu, 04 Feb 2016 15:17:54 +0000 schrieb Saurabh Das <saurabh.das@gmail.com>: > On Thursday, 4 February 2016 at 12:28:39 UTC, Saurabh Das wrote: > > This code: > > [...] > > Update: Simplified, this also doesn't work: > > void main() > { > import std.typecons; > auto tp = tuple(10, false, "hello"); > > auto u0 = tp.slice!(0, tp.length); > auto u1 = tp.slice!(1, tp.length); > auto u2 = tp.slice!(2, tp.length); > > static assert(is(typeof(u0) == Tuple!(int, bool, string))); > static assert(is(typeof(u1) == Tuple!(bool, string))); > static assert(is(typeof(u2) == Tuple!(string))); > > assert(u2[0] == "hello"); > assert(u0[2] == "hello"); > assert(u1[1] == "hello"); // This fails > } > > rdmd erasetype.d > core.exception.AssertError@erasetype.d(16): Assertion failure > > Any ideas? Yes this is a clear bug, I'll report a bug and post the issue number later. -- Marco |
February 04, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Marco Leise | https://issues.dlang.org/show_bug.cgi?id=15645 |
February 05, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Marco Leise | On Thursday, 4 February 2016 at 17:52:16 UTC, Marco Leise wrote:
> https://issues.dlang.org/show_bug.cgi?id=15645
Thank you.
I understood why this is happening from your explanation in the bug report.
|
February 05, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Marco Leise | On Thursday, 4 February 2016 at 17:52:16 UTC, Marco Leise wrote:
> https://issues.dlang.org/show_bug.cgi?id=15645
Is this a possible fixed implementation? :
@property
Tuple!(sliceSpecs!(from, to)) slice(size_t from, size_t to)() @trusted const
if (from <= to && to <= Types.length)
{
auto sliceMixinGenerator()
{
string rv;
for(auto i=from; i<to; ++i)
{
import std.conv : to;
rv ~= "returnValue[" ~ (i-from).to!string ~ "]=field[" ~ i.to!string ~"];";
}
return rv;
}
alias ReturnType = typeof(return);
ReturnType returnValue;
mixin(sliceMixinGenerator());
return returnValue;
}
///
unittest
{
Tuple!(int, string, float, double) a;
a[1] = "abc";
a[2] = 4.5;
auto s = a.slice!(1, 3);
static assert(is(typeof(s) == Tuple!(string, float)));
assert(s[0] == "abc" && s[1] == 4.5);
Tuple!(int, int, long) b;
b[1] = 42;
b[2] = 101;
auto t = b.slice!(1, 3);
static assert(is(typeof(t) == Tuple!(int, long)));
assert(t[0] == 42 && t[1] == 101);
}
I'm unsure about:
1. Removing 'ref' from the return type
2. Adding 'const' to the function signature
3. Is the new implementation less efficient for correctly aligned tuples?
|
February 05, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Saurabh Das | On Friday, 5 February 2016 at 05:18:01 UTC, Saurabh Das wrote:
> [...]
PS: Additionally, '@trusted' can now be substituted with '@safe'.
|
February 05, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Saurabh Das | On Friday, 5 February 2016 at 05:18:01 UTC, Saurabh Das wrote:
[...]
Apologies for spamming. This is an improved implementation:
@property
Tuple!(sliceSpecs!(from, to)) slice(size_t from, size_t to)() @safe const
if (from <= to && to <= Types.length)
{
return typeof(return)(field[from .. to]);
}
///
unittest
{
Tuple!(int, string, float, double) a;
a[1] = "abc";
a[2] = 4.5;
auto s = a.slice!(1, 3);
static assert(is(typeof(s) == Tuple!(string, float)));
assert(s[0] == "abc" && s[1] == 4.5);
Tuple!(int, int, long) b;
b[1] = 42;
b[2] = 101;
auto t = b.slice!(1, 3);
static assert(is(typeof(t) == Tuple!(int, long)));
assert(t[0] == 42 && t[1] == 101);
}
These questions still remain:
> 1. Removing 'ref' from the return type
> 2. Adding 'const' to the function signature
> 3. Is the new implementation less efficient for correctly aligned tuples?
4. @trusted -> @safe?
|
February 05, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Saurabh Das | Am Fri, 05 Feb 2016 05:31:15 +0000 schrieb Saurabh Das <saurabh.das@gmail.com>: > On Friday, 5 February 2016 at 05:18:01 UTC, Saurabh Das wrote: [...] > > Apologies for spamming. This is an improved implementation: > > @property > Tuple!(sliceSpecs!(from, to)) slice(size_t from, size_t > to)() @safe const > if (from <= to && to <= Types.length) > { > return typeof(return)(field[from .. to]); > } > > /// > unittest > { > Tuple!(int, string, float, double) a; > a[1] = "abc"; > a[2] = 4.5; > auto s = a.slice!(1, 3); > static assert(is(typeof(s) == Tuple!(string, float))); > assert(s[0] == "abc" && s[1] == 4.5); > > Tuple!(int, int, long) b; > b[1] = 42; > b[2] = 101; > auto t = b.slice!(1, 3); > static assert(is(typeof(t) == Tuple!(int, long))); > assert(t[0] == 42 && t[1] == 101); > } That's quite concise. I like this. Though 'field' is now called 'expand': // backwards compatibility alias field = expand; > These questions still remain: > > 1. Removing 'ref' from the return type Must happen. 'ref' only worked because of the reinterpreting cast which doesn't work in general. This will change the semantics. Now the caller of 'slice' will deal with a whole new copy of everything in the returned slice instead of a narrower view into the original data. But that's a needed change to fix the bug. > > 2. Adding 'const' to the function signature Hmm. Since const is transitive this may add const to stuff that wasn't const, like in a Tuple!(Object). When you call const slice on that, you would get a Tuple!(const(Object)). I would use inout, making it so that the tuple's original constness is propagated to the result. I.e.: @property inout(Tuple!(sliceSpecs!(from, to))) slice(size_t from, size_t to)() @safe inout > > 3. Is the new implementation less efficient for correctly aligned tuples? Yes, the previous one just added a compile-time known offset to the "this"-pointer. That's _one_ assembly instruction after inlining and optimization. The new one makes a copy of every field. On struct fields this can call the copy constructor "this(this)" which is used for example in reference counting to preform an increment for the copy. On "plain old data" it would simply copy the bit patterns. But that's obviously still less efficient than adding an offset to the pointer. You need two methods if you want to offer the best of both worlds. As soon as your function does not return a pointer or has 'ref' on it, the compiler will provide memory on the stack to hold the result and a copy will occur. That said, sufficiently smart compilers can analyze what's happening and come to the conclusion that when after the copy, the original is no longer used, they can be merged. > 4. @trusted -> @safe? Sounds good, but be aware of the mentioned implications with "this(this)". Copy constructors often need to do unsafe things, so @safe here would disallow them in Tuples. On the other hand recent versions of the front-end should infer attributes for templates, so you can generally omit them and "let the Tuple decide". This mechanism also already adds @nogc, pure, nothrow as possible in both the original and your implementation. (The original code only had @trusted on it because the compiler would always infer the safety as @system due to the pointer casts and @system is close to intolerable for a "high-level" functional programming feature such as tuples. The other attributes should have been inferred correctly.) -- Marco |
February 06, 2016 Re: Is this a bug in std.typecons.Tuple.slice? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Marco Leise | On Friday, 5 February 2016 at 19:16:11 UTC, Marco Leise wrote: >> > 1. Removing 'ref' from the return type > > Must happen. 'ref' only worked because of the reinterpreting cast which doesn't work in general. This will change the semantics. Now the caller of 'slice' will deal with a whole new copy of everything in the returned slice instead of a narrower view into the original data. But that's a needed change to fix the bug. Actually, it's not: https://github.com/D-Programming-Language/phobos/pull/3973 All that is required is to include a little padding at the beginning of the slice struct to make the alignments match. |
Copyright © 1999-2021 by the D Language Foundation