Jump to page: 1 2
Thread overview
[Issue 10850] Inout substituted incorrectly for delegates/fptrs in inout function signature
Sep 13, 2014
Sobirari Muhomori
Sep 13, 2014
timon.gehr@gmx.ch
Sep 13, 2014
Sobirari Muhomori
Sep 13, 2014
timon.gehr@gmx.ch
Sep 13, 2014
Sobirari Muhomori
Sep 13, 2014
Sobirari Muhomori
Sep 13, 2014
Sobirari Muhomori
Sep 13, 2014
Sobirari Muhomori
Sep 14, 2014
Kenji Hara
Sep 15, 2014
Sobirari Muhomori
Sep 15, 2014
Stewart Gordon
Sep 16, 2014
Sobirari Muhomori
Sep 16, 2014
Sobirari Muhomori
Sep 22, 2014
Stewart Gordon
Sep 23, 2014
Sobirari Muhomori
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #2 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
(In reply to timon.gehr from comment #0)
> git head:
> 
> Since apparently inout now refers to the outermost inout function

What? How? That looks incorrect. inout should be applied to data passed to the inout function, not to delegate signatures, those are independent.

> should also be substituted for delegates inside the function signature:
> 
> inout(int)* delegate(inout(int)*) foo(ref inout(int) x){
>     inout(int)* bar(inout(int)*) { return &x; }
>     return &bar;
> }

If one wants a really really stupid fix for nested inout functions, then inout data outside of the nested function should be treated as const.

> immutable(int) x;
> static assert(is(typeof(foo(x))==immutable(int)* delegate(immutable(int)*)));

I'd say, the return type should be inout delegate, converting it to immutable is possible, but unnecessarily restrictive.


assert(foo(&a,x=>x) is &a); looks like duplicate of issue 11772 (works for
function, but not for delegate)

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

timon.gehr@gmx.ch changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major

--- Comment #3 from timon.gehr@gmx.ch ---
(In reply to Sobirari Muhomori from comment #2)
> (In reply to timon.gehr from comment #0)
> > git head:
> > 
> > Since apparently inout now refers to the outermost inout function
> 
> What? How? That looks incorrect.

What are you referring to? The current behaviour of DMD is indeed to consider 'inout' bound to the outermost inout function signature, this is why the following code does not compile:

void foo(ref inout(int) x){
    inout(int)* bar(inout(int)*) { return &x; }
    int y;
    bar(&y); // error
}


> inout should be applied to data passed to
> the inout function, not to delegate signatures, those are independent.
> ...

Well, no they are not.

It appears that DMD has been changed in the meantime to treat inout within delegate signatures within function/delegate signatures as 'const'. However, this is unsound:

inout(int)** delegate(inout int) foo(inout(int)** x){
    inout(int)** bar(inout int){ return x; }
    return &bar;
}

void main(){
    immutable(int) x;
    immutable(int)* ipx=&x;
    immutable(int)** ippx=&ipx;
    const(int)** cppx=foo(ippx)(2);
    int y=3; int* py=&y;
    *cppx=py;
    ipx=*ippx;
    assert(ipx is &y);
    static assert(is(typeof(*ipx)==immutable));
    assert(*ipx==3);
    y=4;
    assert(*ipx==4);
}

Increasing importance to major, because this issue causes type unsoundness.

> > should also be substituted for delegates inside the function signature:
> > 
> > inout(int)* delegate(inout(int)*) foo(ref inout(int) x){
> >     inout(int)* bar(inout(int)*) { return &x; }
> >     return &bar;
> > }
> 
> If one wants a really really stupid fix for nested inout functions, then inout data outside of the nested function should be treated as const. ...

All of these ad-hoc fixes are of a certain minimum stupidity because there is no right answer here (more precisely, the 'right' answer would need to change the surface syntax, because there would need to be more than one inout qualifier). This report is assuming that the fix that Kenji Hara implemented in DMD is the official fix.

In any case, this suggestion (as I understand it) is indeed particularly stupid
as it is unsound:

const(int)** foo(inout(int)** x){
    // inout treated as const from nested context:
    const(int)** bar(){ return x; }
    return bar();
}

(This is the same type coercion underlying the previous proof of unsoundness.)

> > immutable(int) x;
> > static assert(is(typeof(foo(x))==immutable(int)* delegate(immutable(int)*)));
> 
> I'd say, the return type should be inout delegate, converting it to
> immutable is possible, but unnecessarily restrictive.
> ...

Unnecessary how?

inout(int)* delegate(inout(int)*) foo(ref inout(int) x){
    inout(int)* bar(inout(int)*) { return &x; }
    return &bar;
}

void main(){
    immutable(int) x=2;
    auto dg=cast(inout(int)* delegate(inout(int)*))foo(x);
    int y;
    assert(dg(&y) is &x);
    *dg(&y)=3; // modification of immutable data
    assert(x!=*&x); // passes with DMD 2.066.0
}

I.e. everything else staying the same, the typing rules you propose are still unsound.

> 
> assert(foo(&a,x=>x) is &a); looks like duplicate of issue 11772 (works for
> function, but not for delegate)

If anything, then issue 11772 is the duplicate.

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #4 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
(In reply to Sobirari Muhomori from comment #2)
> > should also be substituted for delegates inside the function signature:
> > 
> > inout(int)* delegate(inout(int)*) foo(ref inout(int) x){
> >     inout(int)* bar(inout(int)*) { return &x; }
> >     return &bar;
> > }
> 
> If one wants a really really stupid fix for nested inout functions, then inout data outside of the nested function should be treated as const.

An implementation, which should be technically easy is to preprocess the type of an outer variable at the point of access in nested function and convert its inout qualifier to const. So &x expression inside `bar` function will have const(int)* type and will not implicitly convert to inout(int)* return type.

Illustration, why `bar` shouldn't compile:

inout(int)* delegate(inout(int)*) foo(ref inout(int) x){
    inout(int)* bar(inout(int)*) { return &x; }
    immutable int n;
    immutable int* n1 = bar(&n); //ok for bar's signature
    return &bar;
}

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #5 from timon.gehr@gmx.ch ---
(In reply to Sobirari Muhomori from comment #4)
> ...
> An implementation, which should be technically easy is to preprocess the
> type of an outer variable at the point of access in nested function and
> convert its inout qualifier to const. ...

As I have pointed out, this approach is unsound.

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #6 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
(In reply to timon.gehr from comment #3)
> What are you referring to? The current behaviour of DMD is indeed to consider 'inout' bound to the outermost inout function signature

I mean, it shouldn't work that way.

> All of these ad-hoc fixes are of a certain minimum stupidity because there is no right answer here (more precisely, the 'right' answer would need to change the surface syntax, because there would need to be more than one inout qualifier).

The right answer is the type system with existing syntax should be sound even if it can be sound with different syntax.

> In any case, this suggestion (as I understand it) is indeed particularly
> stupid as it is unsound:
> 
> const(int)** foo(inout(int)** x){
>     // inout treated as const from nested context:
>     const(int)** bar(){ return x; }
>     return bar();
> }
> 
> (This is the same type coercion underlying the previous proof of
> unsoundness.)

Indeed, it's a const casting and should obey const casting rules as described in issue 4251: if conversion happens, everything beyond the first indirection should become const. I thought, inout already takes care of that, but it happened to be more flexible.

> > assert(foo(&a,x=>x) is &a); looks like duplicate of issue 11772 (works for
> > function, but not for delegate)
> 
> If anything, then issue 11772 is the duplicate.

That issue is about discrepancy between function and delegate behavior, it has nothing to do with inout being bound to outer function.

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #7 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
Binding inout to the outer function is probably an even easier implementation, than what I propose. But restricting.

Also:
---
void foo(ref inout(int) x){
    inout(int)* bar(inout(int)*) { return &x; }
    int y;
    bar(&y); // error
}
---
Error: modify inout to mutable is not allowed inside inout function
---
The error message should be probably "cannot implicitly convert expression (&
y) of type int* to inout(int)*" like for any other attempt co convert mutable
to inout.

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #8 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
If different inout functions are independent:
---
inout(int)* delegate(inout(int)*) f(inout(int)* x)
{
    inout(int)* g(inout(int)* y) { return y; }
    //outer inout variables are seen as const
    //inout(int)* g(inout(int)* y) { return x; }
    int z;
    int* a = g(&z); //ok like any other inout function
    return &g;
}

void h()
{
    int x;
    //received true inout delegate
    inout(int)* delegate(inout(int)*) q = f(&x);
}
---


If inout is bound to the outer function:
---
inout(int)* delegate(inout(int)*) f(inout(int)* x)
{
    //work freely with local inout variables
    inout(int)* g(inout(int)* y) { return x; }
    int z;
    //int* a = g(&z); //can't convert mutable to inout
    return &g;
}

void r(inout(int)* x, inout(int)* delegate(inout(int)*) dg)
{
    dg(x);
}

void h()
{
    int x;
    //can't receive inout delegate
    int* delegate(int*) q = f(&x);

    //function can indirectly mutate its inout argument
    r(&x, (int* y){ ++*y; });
    assert(x==1);
}
---

Not sure, which is better.

--
September 13, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

--- Comment #9 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
Hmm... now that I think of it, binding inout to the outer function probably has more merits than independent functions.

--
September 14, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=10758

--- Comment #10 from Kenji Hara <k.hara.pg@gmail.com> ---
Related: issue 10758

--
September 15, 2014
https://issues.dlang.org/show_bug.cgi?id=10850

Sobirari Muhomori <dfj1esp02@sneakemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2573

--- Comment #11 from Sobirari Muhomori <dfj1esp02@sneakemail.com> ---
Tests for binding:

1. Should pass:
---
struct S {
    int[] a;
    int opApply(scope int delegate(ref inout int) dg) inout {
        foreach(ref x;a) if(auto r=dg(x)) return r;
        return 0;
    }
}

void main() {
    auto s=S([1,2,3]);
    foreach(ref x;s) x++; // ok
}
---

2. Should not compile:
---
struct S {
    int[] a;
    int opApply(scope int delegate(ref inout int) dg) inout {
        foreach(ref x;a) if(auto r=dg(x)) return r;
        return 0;
    }
}

void main() {
    const(S) s=S([1,2,3]);
    foreach(ref x;s) {
        import std.stdio;
        writeln(x);
        x++; //error
    }
}
---

--
« First   ‹ Prev
1 2