On Friday, 5 February 2016 at 19:16:11 UTC, Marco Leise wrote:
> Am Fri, 05 Feb 2016 05:31:15 +0000
> schrieb Saurabh Das <saurabh.das@gmail.com>:
>
> [...]

That is enlightening. I have updated the PR at https://github.com/D-Programming-Language/phobos/pull/3975 to incorporate these changes.

Am Sat, 06 Feb 2016 04:28:17 +0000
schrieb tsbockman <thomas.bockman@gmail.com>:

> 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.

I don't want to sound dismissive, but when that thought came
to my mind I considered it unacceptable that the type of
Tuple!(int, bool, string).slice(1, 3) would be something
different than Tuple!(bool, string). In your case
Tuple!(TuplePad!4LU, bool, string). That's just a matter of
least surprise when comparing the types.

I'll let others decide, since I never used tuple slices.

-- 
Marco

On Saturday, 6 February 2016 at 06:34:05 UTC, Marco Leise wrote:
> I don't want to sound dismissive, but when that thought came
> to my mind I considered it unacceptable that the type of
> Tuple!(int, bool, string).slice(1, 3) would be something
> different than Tuple!(bool, string). In your case
> Tuple!(TuplePad!4LU, bool, string). That's just a matter of
> least surprise when comparing the types.
>
> I'll let others decide, since I never used tuple slices.

I'm not sure which approach is ultimately better, but aside from the performance implications, your "needed change" could break a lot of valid code in the wild - or it might break none; it really just depends on whether anyone actually *uses* the `ref`-ness of the `Tuple.slice` return type.

(It appears that Phobos, at least, does not. But there is no guarantee that the rest of the world is using `Tuple` only in the ways that Phobos does.)

Leaving aside bizarre meta-programming stuff (because then *anything* is a breaking change), my PR does not break any code, except that which was already broken: the type of the slice is only different in those cases where it *has* to be, for alignment reasons; otherwise it remains the same as it was before.

On Saturday, 6 February 2016 at 06:34:05 UTC, Marco Leise wrote:
> [...]

I should also point out that, since there is no way to actually find out whether anyone is using the `ref`-ness of the return type in the wild, the approach that you and Saurabh Das are taking really ought to include changing the symbol name and deprecating the old one.

Otherwise you could introduce subtle bugs into previously valid code; not every significant effect of removing `ref` will cause an error message at compile time *or* run time - some will just silently change the behaviour of the program, which is awful.

On Saturday, 6 February 2016 at 08:01:20 UTC, tsbockman wrote:
> On Saturday, 6 February 2016 at 06:34:05 UTC, Marco Leise wrote:
>> [...]
>
> I should also point out that, since there is no way to actually find out whether anyone is using the `ref`-ness of the return type in the wild, the approach that you and Saurabh Das are taking really ought to include changing the symbol name and deprecating the old one.
>
> Otherwise you could introduce subtle bugs into previously valid code; not every significant effect of removing `ref` will cause an error message at compile time *or* run time - some will just silently change the behaviour of the program, which is awful.

I think we should add a static assert to slice to ensure that the current implementation is not used in a case where the alignment doesn't match. This is better than failing without any warning.

We could add new (differently named) functions for slicing non-aligned tuples.

I agree that my approach of removing the ref may break existing code, so if introduced, it should be named differently.

I am not comfortable with tuple(42, true, "abc").slice(1, 3) being different in type from tuple(true, " abc").

On Saturday, 6 February 2016 at 08:47:01 UTC, Saurabh Das wrote:
> I think we should add a static assert to slice to ensure that the current implementation is not used in a case where the alignment doesn't match. This is better than failing without any warning.

If we pursue the deprecation route, I agree that this is a necessary step.

> We could add new (differently named) functions for slicing non-aligned tuples.
>
> I agree that my approach of removing the ref may break existing code, so if introduced, it should be named differently.
>
> I am not comfortable with tuple(42, true, "abc").slice(1, 3) being different in type from tuple(true, " abc").

Why? What practical problem does this cause?

Am Sat, 06 Feb 2016 07:57:08 +0000
schrieb tsbockman <thomas.bockman@gmail.com>:

> On Saturday, 6 February 2016 at 06:34:05 UTC, Marco Leise wrote:
> > I don't want to sound dismissive, but when that thought came
> > to my mind I considered it unacceptable that the type of
> > Tuple!(int, bool, string).slice(1, 3) would be something
> > different than Tuple!(bool, string). In your case
> > Tuple!(TuplePad!4LU, bool, string). That's just a matter of
> > least surprise when comparing the types.
> >
> > I'll let others decide, since I never used tuple slices.
> 
> I'm not sure which approach is ultimately better, but aside from the performance implications, your "needed change" could break a lot of valid code in the wild - or it might break none; it really just depends on whether anyone actually *uses* the `ref`-ness of the `Tuple.slice` return type.

True.

> (It appears that Phobos, at least, does not. But there is no
> guarantee that the rest of the world is using `Tuple` only in the
> ways that Phobos does.)
> Leaving aside bizarre meta-programming stuff (because then
> *anything* is a breaking change), my PR does not break any code,
> except that which was already broken: the type of the slice is
> only different in those cases where it *has* to be, for alignment
> reasons; otherwise it remains the same as it was before.

I understand that. We just have a different perspective on the problem. Your priorities:

- don't break what's not broken
- .slice! lends on opSlice and should return by ref

My priorities:

- type of .slice! should be as if constructing with same
  values from scratch
- keep code additions in Phobos to a minimum

Why do I insist on the return type? Because surprisingly simple code breaks if it doesn't match. Not everything can be covered by runtime conversions in D. It still took me a while to come up with something obvious:

	uint[Tuple!(uint, ulong)] hash;

	auto tup = tuple(1u, 2u, 3UL);

	hash[tup.slice!(1, 3)] = tup[0];

                     compiles?  works?
original Tuple     : yes        no
Saurabh Das changes: yes        yes
your changes       : no         no

What I like most about your proposal is that it doesn't break any existing code that wasn't broken before. That can't be emphasized enough.

-- 
Marco

Am Sat, 06 Feb 2016 11:02:37 +0000
schrieb tsbockman <thomas.bockman@gmail.com>:

> On Saturday, 6 February 2016 at 08:47:01 UTC, Saurabh Das wrote:
> > I think we should add a static assert to slice to ensure that the current implementation is not used in a case where the alignment doesn't match. This is better than failing without any warning.
> 
> If we pursue the deprecation route, I agree that this is a necessary step.

That would hurt the least, yes. It's more like a .dup with start and end parameter then.

> > We could add new (differently named) functions for slicing non-aligned tuples.
> >
> > I agree that my approach of removing the ref may break existing code, so if introduced, it should be named differently.
> >
> > I am not comfortable with tuple(42, true, "abc").slice(1, 3) being different in type from tuple(true, " abc").
> 
> Why? What practical problem does this cause?

For me it is mostly a gut feeling. Historically types mimicking other types have often evoked trouble for example in generic functions or concretely - if you look at my other post - in D's AA implementation.

-- 
Marco

On Sunday, 7 February 2016 at 02:11:15 UTC, Marco Leise wrote:
> Why do I insist on the return type? Because surprisingly simple code breaks if it doesn't match. Not everything can be covered by runtime conversions in D. It still took me a while to come up with something obvious:
>
> 	uint[Tuple!(uint, ulong)] hash;
>
> 	auto tup = tuple(1u, 2u, 3UL);
>
> 	hash[tup.slice!(1, 3)] = tup[0];
>
>                      compiles?  works?
> original Tuple     : yes        no
> Saurabh Das changes: yes        yes
> your changes       : no         no

Thank you for the example.

If multiple alias this ever makes it into the language (see https://wiki.dlang.org/DIP66 and https://github.com/D-Programming-Language/dmd/pull/3998), this could be fixed quite easily. But, I do not see any way to fix it with the tools currently available.

On Sunday, 7 February 2016 at 02:11:15 UTC, Marco Leise wrote:
> I understand that. We just have a different perspective on the problem. Your priorities:
>
> - don't break what's not broken
> - .slice! lends on opSlice and should return by ref
>
> My priorities:
>
> - type of .slice! should be as if constructing with same
>   values from scratch
> - keep code additions in Phobos to a minimum
>
> Why do I insist on the return type? Because surprisingly simple code breaks if it doesn't match. Not everything can be covered by runtime conversions in D.

I think the key question is, do users care about being able to modify the original `Tuple` instance indirectly through `slice`?

If yes, then the only workable solutions I can see are:

1) My current proposal (insert a hidden padding member at the beginning of the slice `Tuple`)

2) Don't return a `Tuple` at all - return a dedicated `TupleSlice` type that is implicitly convertible to `Tuple`. This approach would work with the example you came up with, but implementing `TupleSlice` well could be very complex, I think.

If not, then I have no fundamental objection to Saurabh Das' approach, although I think the PR needs work.

We should start a new thread in "General" to ask whether people care about the `ref`-ness of `Tuple` slices is really the deciding factor.