Is this a bug in std.typecons.Tuple.slice?

[Marco Leise via Digitalmars-d-learn]

Am Fri, 05 Feb 2016 05:31:15 +0000
schrieb Saurabh Das <saurabh.das at 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