Jump to page: 1 24  
Page
Thread overview
Is this a bug in std.typecons.Tuple.slice?
Feb 04, 2016
Saurabh Das
Feb 04, 2016
Saurabh Das
Feb 04, 2016
Marco Leise
Feb 04, 2016
Marco Leise
Feb 05, 2016
Saurabh Das
Feb 05, 2016
Saurabh Das
Feb 05, 2016
Saurabh Das
Feb 05, 2016
Saurabh Das
Feb 05, 2016
Marco Leise
Feb 06, 2016
tsbockman
Feb 06, 2016
Marco Leise
Feb 06, 2016
tsbockman
Feb 07, 2016
Marco Leise
Feb 07, 2016
tsbockman
Feb 07, 2016
tsbockman
Feb 07, 2016
tsbockman
Feb 07, 2016
Saurabh Das
Feb 09, 2016
tsbockman
Feb 09, 2016
Marco Leise
Feb 09, 2016
tsbockman
Feb 09, 2016
Marc Schütz
Feb 09, 2016
Marc Schütz
Feb 09, 2016
Marc Schütz
Feb 10, 2016
tsbockman
Feb 10, 2016
Saurabh Das
Feb 06, 2016
tsbockman
Feb 06, 2016
Saurabh Das
Feb 06, 2016
tsbockman
Feb 07, 2016
Marco Leise
Feb 06, 2016
Saurabh Das
February 04, 2016
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
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
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
https://issues.dlang.org/show_bug.cgi?id=15645
February 05, 2016
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
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
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
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
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
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.
« First   ‹ Prev
1 2 3 4