Bug 10850 – Inout substituted incorrectly for delegates/fptrs in inout function signature

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

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.

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

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

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

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

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

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.

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.

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

Related: issue 10758

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

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.

A good reason for binding: http://forum.dlang.org/post/[email protected] (In reply to Kenji Hara from comment #10) > Related: issue 10758 Does it have a test for voldemort types returned from a function?

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

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

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

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17606 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB