I grepped phobos for "inout" and picked a heavy one. Not even cherry picking here. You be the judges.

Before:

inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
{
    alias U = inout(T);
    static U* max(U* a, U* b) nothrow { return a > b ? a : b; }
    static U* min(U* a, U* b) nothrow { return a < b ? a : b; }

    auto b = max(r1.ptr, r2.ptr);
    auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
    return b < e ? b[0 .. e - b] : null;
}

After:

auto overlap(T, U)(T[] r1, U[] r2) @trusted pure nothrow
if (is(typeof(r1.ptr < r2.ptr) == bool))
{
    import std.algorithm : min, max;
    auto b = max(r1.ptr, r2.ptr);
    auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
    return b < e ? b[0 .. e - b] : null;
}

Who wore it better?


Andrei

https://github.com/D-Programming-Language/phobos/pull/4201 -- Andrei

Since it is a template: Why these attributes: @trusted pure nothrow ?

On 04/15/2016 01:31 PM, Namespace wrote:
> Since it is a template: Why these attributes: @trusted pure nothrow ?

@trusted is not inferrable, the others are type-independent and nice for the documentation. -- Andrei

On 4/15/16 1:24 PM, Andrei Alexandrescu wrote:
> I grepped phobos for "inout" and picked a heavy one. Not even cherry
> picking here. You be the judges.
>
> Before:
>
> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
> {
>      alias U = inout(T);
>      static U* max(U* a, U* b) nothrow { return a > b ? a : b; }
>      static U* min(U* a, U* b) nothrow { return a < b ? a : b; }
>
>      auto b = max(r1.ptr, r2.ptr);
>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>      return b < e ? b[0 .. e - b] : null;
> }
>
> After:
>
> auto overlap(T, U)(T[] r1, U[] r2) @trusted pure nothrow
> if (is(typeof(r1.ptr < r2.ptr) == bool))
> {
>      import std.algorithm : min, max;
>      auto b = max(r1.ptr, r2.ptr);
>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>      return b < e ? b[0 .. e - b] : null;
> }
>
> Who wore it better?

inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
{
    import std.algorithm: min, max;
    auto b = max(r1.ptr, r2.ptr);
    auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
    return b < e ? b[0 .. e - b] : null;
}

-Steve

On 4/15/16 2:46 PM, Steven Schveighoffer wrote:
> On 4/15/16 1:24 PM, Andrei Alexandrescu wrote:
>> I grepped phobos for "inout" and picked a heavy one. Not even cherry
>> picking here. You be the judges.
>>
>> Before:
>>
>> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
>> {
>>      alias U = inout(T);
>>      static U* max(U* a, U* b) nothrow { return a > b ? a : b; }
>>      static U* min(U* a, U* b) nothrow { return a < b ? a : b; }
>>
>>      auto b = max(r1.ptr, r2.ptr);
>>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>>      return b < e ? b[0 .. e - b] : null;
>> }
>>
>> After:
>>
>> auto overlap(T, U)(T[] r1, U[] r2) @trusted pure nothrow
>> if (is(typeof(r1.ptr < r2.ptr) == bool))
>> {
>>      import std.algorithm : min, max;
>>      auto b = max(r1.ptr, r2.ptr);
>>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>>      return b < e ? b[0 .. e - b] : null;
>> }
>>
>> Who wore it better?
>
> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
> {
>      import std.algorithm: min, max;
>      auto b = max(r1.ptr, r2.ptr);
>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>      return b < e ? b[0 .. e - b] : null;
> }

Is that better or worse than the one without inout? -- Andrei

On 4/15/16 2:48 PM, Andrei Alexandrescu wrote:
> On 4/15/16 2:46 PM, Steven Schveighoffer wrote:
>> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure nothrow
>> {
>>      import std.algorithm: min, max;
>>      auto b = max(r1.ptr, r2.ptr);
>>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>>      return b < e ? b[0 .. e - b] : null;
>> }
>
> Is that better or worse than the one without inout? -- Andrei

Better. It generates one implementation for all 9 combinations of mutability. Yours generates 9 identical binary functions.

And yours possibly depends on a bug: https://issues.dlang.org/show_bug.cgi?id=15930

-Steve

On 04/15/2016 03:13 PM, Steven Schveighoffer wrote:
> On 4/15/16 2:48 PM, Andrei Alexandrescu wrote:
>> On 4/15/16 2:46 PM, Steven Schveighoffer wrote:
>>> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure
>>> nothrow
>>> {
>>>      import std.algorithm: min, max;
>>>      auto b = max(r1.ptr, r2.ptr);
>>>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>>>      return b < e ? b[0 .. e - b] : null;
>>> }
>>
>> Is that better or worse than the one without inout? -- Andrei
>
> Better. It generates one implementation for all 9 combinations of
> mutability. Yours generates 9 identical binary functions.

A valid but weak argument. There's been long talk about eliminating binary identical functions in the front end (some linkers already do it). That would be the real solution that would help cases unrelated to inout, too.

> And yours possibly depends on a bug:
> https://issues.dlang.org/show_bug.cgi?id=15930

Red herring. Fixing the bug shouldn't break that code.


Andrei

On 4/15/16 4:05 PM, Andrei Alexandrescu wrote:
> On 04/15/2016 03:13 PM, Steven Schveighoffer wrote:
>> On 4/15/16 2:48 PM, Andrei Alexandrescu wrote:
>>> On 4/15/16 2:46 PM, Steven Schveighoffer wrote:
>>>> inout(T)[] overlap(T)(inout(T)[] r1, inout(T)[] r2) @trusted pure
>>>> nothrow
>>>> {
>>>>      import std.algorithm: min, max;
>>>>      auto b = max(r1.ptr, r2.ptr);
>>>>      auto e = min(r1.ptr + r1.length, r2.ptr + r2.length);
>>>>      return b < e ? b[0 .. e - b] : null;
>>>> }
>>>
>>> Is that better or worse than the one without inout? -- Andrei
>>
>> Better. It generates one implementation for all 9 combinations of
>> mutability. Yours generates 9 identical binary functions.
>
> A valid but weak argument. There's been long talk about eliminating
> binary identical functions in the front end (some linkers already do
> it). That would be the real solution that would help cases unrelated to
> inout, too.

The main argument is this:

auto overlap(T, U)(T[] r1, U[] r2) @trusted pure nothrow

Can you tell, does overlap modify any data in r1 or r2?

If you find such advertisement useless, you of course do not need inout or const.

>> And yours possibly depends on a bug:
>> https://issues.dlang.org/show_bug.cgi?id=15930
>
> Red herring. Fixing the bug shouldn't break that code.

I don't know what the bug is. That's why I said "possibly." I was surprised your code compiled with both const/mutable parameters, but then I found the bug. It's possible it could be fixed and become correct. I'm not sure.

-Steve

On 04/15/2016 04:16 PM, Steven Schveighoffer wrote:
> If you find such advertisement useless, you of course do not need inout
> or const.

Let's not exaggerate by putting them together. -- Andrei