Inout substituted incorrectly for delegates/fptrs in inout function signature

[dlangBugzillaToGithub]

timon.gehr reported this on 2013-08-19T02:49:22Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=10850

CC List

• Stewart Gordon

Description

git head:

Since apparently inout now refers to the outermost inout function, inout 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;
}
immutable(int) x;
static assert(is(typeof(foo(x))==immutable(int)* delegate(immutable(int)*)));

(Also note that none of the recent language updates towards type safe inout checking were documented.)

[dlangBugzillaToGithub]

timon.gehr commented on 2013-08-19T02:57:40Z

Other test case:

inout(int)* foo(inout(int)* a, inout(int)* delegate(inout(int)*) dg){
    return dg(a);
}
inout(int)* bar(inout(int)* a, inout(int)* delegate(inout(int)*) dg){
    auto x = dg(a);
    int* y;
    dg(y);
    return x;
}	
void main(){
    immutable int a;
    assert(foo(&a,x=>x) is &a); // error
    static assert(is(typeof(foo(&a,x=>x))==immutable(int)*)); // error
    assert(foo(&a,(immutable(int)* x)=>x) is &a); // error
}

This should compile.

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T17:20:14Z

(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)

[dlangBugzillaToGithub]

timon.gehr commented on 2014-09-13T18:11:03Z

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

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T19:36:58Z

(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;
}

[dlangBugzillaToGithub]

timon.gehr commented on 2014-09-13T19:42:57Z

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

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T20:32:15Z

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

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T21:24:39Z

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.

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T21:47:15Z

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.

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-13T22:14:52Z

Hmm... now that I think of it, binding inout to the outer function probably has more merits than independent functions.

[dlangBugzillaToGithub]

k.hara.pg commented on 2014-09-14T15:06:38Z

Related: issue 10758

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-15T12:04:15Z

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

[dlangBugzillaToGithub]

smjg commented on 2014-09-15T18:22:41Z

The spec doesn't comment one way or the other.  There was a discussion once upon a time about it, but I don't remember it coming to any conclusion.  I'll try and dig it up when I get the chance.

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-16T10:43:03Z

A good reason for binding: http://forum.dlang.org/post/lv441t$23qa$1@digitalmars.com

(In reply to Kenji Hara from comment #10)
> Related: issue 10758

Does it have a test for voldemort types returned from a function?

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-16T10:46:19Z

Another complication of binding is that `inout` is going to be allowed in fields of nested types, while normally it's disallowed.

[dlangBugzillaToGithub]

smjg commented on 2014-09-22T16:01:38Z

(In reply to Sobirari Muhomori from comment #13)
> Does it have a test for voldemort types returned from a function?

What's a voldemort type?

[dlangBugzillaToGithub]

dfj1esp02 commented on 2014-09-23T11:05:25Z

Struct nested in a function. Called so because it can't be named outside function.