Bug 7378 – inout constructors do not properly resolve to immutable.

Status
REOPENED
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2012-01-27T06:44:10Z
Last change time
2024-12-13T17:58:04Z
Keywords
wrong-code
Assigned to
No Owner
Creator
Steven Schveighoffer
Moved to GitHub: dmd#17538 →

Comments

Comment #0 by schveiguy — 2012-01-27T06:44:10Z
The following code: import std.stdio; struct T { int *data; this(inout(int)* d) inout {this.data = d;} } void main() { int x; const int xc; immutable int xi; writeln(typeof(T(&x)).stringof); writeln(typeof(T(&xc)).stringof); writeln(typeof(T(&xi)).stringof); } outputs: T const(T) const(T) It should output: T const(T) immutable(T) I suspect the issue is that the constructor's this pointer is being treated as a parameter instead of the result, and it's implicitly declared as mutable. If you consider that in a constructor, the return value is a newly constructed T struct, the 'this' parameter is semantically the return value. So what should happen is the inout resolution mechanism should treat the parameters to the constructor as the only parameters, and the implicit this parameter as the return value.
Comment #1 by maximzms — 2013-06-07T02:08:30Z
Since this pull request was merged https://github.com/D-Programming-Language/dmd/pull/1726 one must specify the qualifier explicitly: -------------------- import std.stdio; struct T { int *data; this(inout(int)* d) inout {this.data = d;} } void main() { int x; const int xc; immutable int xi; writeln(typeof(T(&x)).stringof); writeln(typeof(const T(&xc)).stringof); writeln(typeof(immutable T(&xi)).stringof); } -------------------- So now inout constructors are resolved manually.
Comment #2 by schveiguy — 2013-06-10T08:44:33Z
Reopening, I don't agree with the pulled request. An inout function has to have a return value based on the input values. This pull is for some reason *requiring* you to dictate what the return type should be, when it's clear from the parameters. Look at the following function: inout(int) *foo(inout int *x, inout int *y) { return *x < *y ? x : y; } int x; immutable int ix; We have inout on the return value, inout on the parameters. The return value's inout is dictated by the parameters, it's not dictated manually. You don't write (or have to write): const foo(&x, &ix); You just write: foo(&x, &ix); And the return value is dictated by the parameters (as const(int) *). But if we look at the patch, it requires the verbose redundant form. In this case, inout is not doing what it was meant to do, it's a manually-defined wildcard. And then YOU have to fulfill the promise with the parameters you pass in. While dictating the return type might be a valid use case, it's not the only use case, and significantly diminishes the value of inout. I postulate that the constructor call is simply another function call, and that T() doesn't correspond to "the return type is mutable T", it corresponds to "call T's constructor and let it dictate what to return, based on the parameters". In other words, the function attribute is a property of the RETURN type, not the 'this' parameter. In actuality, the 'this' parameter is a 'unique' type, readily changeable into inout/const/immutable/whatever because it's simply raw data that hasn't yet been referenced. The semantics inside the function follow that (members are mutable even though 'this' is inout). The newly pulled patch makes this overly restrictive. There is no reason to require that immutable be specified on the constructor call, when the only possible valid type modifier is immutable. Inout is supposed to alleviate that requirement. I look at a constructor like this: this(inout(int) *_x) inout { ... } To be interpreted in the compiler like this: inout(T) __ctor(inout(int) *_x) { unique(inout(T)) this; // unique is defined as 'head-mutable until read' ... return this; } My expectation is for: auto t1 = T(&ix); // type of t1 is immutable(T) auto t2 = T(&x); // type of t2 is T auto t3 = const T(&ix); // type of t3 is const(T) (ok to cast immutable(T) to const(T) ) auto t4 = immutable T(&ix); // type of t4 is immutable(T) (equivalent to t1 case) const t5 = T(&ix); // type of t5 is const(T) const t6 = immutable T(&ix); // type of t6 is const(T) // failing calls auto f1 = immutable T(&x); // error, cannot cast T to immutable(T) implicitly immutable f2 = T(&x); // same thing immutable f3 = const T(&cx); // error cannot cast const(T) to immutable(T) implicitly T f4 = T(&ix) // error, cannot cast immutable(T) to T From Kenji's pull request: S([1, 2, 3].idup) should compile, and return an immutable(S). It then cannot be assigned to a mutable S, and that is fine. For what it's worth, I also think this should be the case for non-inout constructors: struct T { int *x; this(int *_x) { x = _x;} this(immutable(int) *_x) immutable { x = _x;} } immutable int ix; auto t = T(&ix); // typeof(t) == immutable(T) CC'ing kenji so he is aware of this view and can agree/disagree/clarify.
Comment #3 by aziz.koeksal — 2013-12-07T10:23:16Z
This used to work for me and I agree with Steven. I think the bug type should be changed to "regression". Hope this will be fixed in 2.064.
Comment #4 by k.hara.pg — 2013-12-07T22:16:21Z
(In reply to comment #3) > This used to work for me and I agree with Steven. > > I think the bug type should be changed to "regression". Hope this will be fixed > in 2.064. Don't mark this issue as a regression. Before my patch applied, qualified constructor (including a constructor that is qualified with inout) had not been defined properly. So _nothing_ used to work. Therefore, this is still an enhancement. Honestly, current inout constructor and unique constructor definition has a serious type-system hole that is presented in the bug 11503. I'm planning fixing the issues by redesigning the definitions of inout/unique constructor - the base concept would be same as the base of DIP49.
Comment #5 by k.hara.pg — 2013-12-18T21:36:59Z
(In reply to comment #4) > Honestly, current inout constructor and unique constructor definition has a > serious type-system hole that is presented in the bug 11503. I'm planning > fixing the issues by redesigning the definitions of inout/unique constructor - > the base concept would be same as the base of DIP49. I opened DIP53 to redesign qualified constructor definition. http://wiki.dlang.org/DIP53 Steven, is that resolve this issue?
Comment #6 by robert.schadek — 2024-12-13T17:58:04Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17538 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB