Bug 9506 – When using alias this, writeln modifies its argument

The function writeln is not expected to modify its arguments, just print them. However when a class uses alias this, writeln clears the member variable in question. Example: import std.stdio; class test { uint[] _p; alias _p this; this() { _p = [1, 2]; } } void main() { test t = new test(); writeln(t); writeln(t); } The expected output is [1, 2] [1, 2] but instead the program outputs [1, 2] []

This must be some kind of codegen bug, I can't recreate this on win32. I doubt any code in writeln (or rather format) tries to do modifications like that.

(In reply to comment #1) > This must be some kind of codegen bug, I can't recreate this on win32. I doubt > any code in writeln (or rather format) tries to do modifications like that. I suppose you're right about codegen. This bug is in v2.060 on OS X. I just tried v2.061, it works correctly.

This should be closed if it can no longer be reproduced.

I've tested this across several compiler versions on win32: 2.058: can't compile due to template instance error 2.059: broken: [1, 2] [ ] 2.060: broken: [1, 2] [ ] 2.061: ok: [1, 2] [1 2] 2.062: ok: [1, 2] [1 2] 2.063: broken: [1, 2] [ ] Even if it was fixed at some point it looks like no test-cases were added, the bug is back.

Gah, we missed our window.

Can reproduce on linux. I haven't found any codegen bug, but value modification. Old value = 2 //length of array New value = 1 0x000000000043a3e6 in std.array.__T8popFrontTkZ.popFront() (a=0x7ffff7ed8ff0) at /usr/include/d/dmd/phobos/std/array.d:451 451 a = a[1 .. $]; (gdb) bt #0 0x000000000043a3e6 in std.array.__T8popFrontTkZ.popFront() ( a=0x7ffff7ed8ff0) at /usr/include/d/dmd/phobos/std/array.d:451 #1 0x000000000043af8c in std.format.__T11formatRangeTS3std5stdio4File17LockingTextWriterTC4main4testTaZ.formatRange() (f=<error reading variable>, val=0x7fffffffd808, w=0x7fffffffd820) at /usr/include/d/dmd/phobos/std/format.d:2155 #2 0x000000000043a312 in std.format.__T11formatValueTS3std5stdio4File17LockingTextWriterTC4main4testTaZ.formatValue() (f=<error reading variable>, val=0x7ffff7ed8fe0, w=...) at /usr/include/d/dmd/phobos/std/format.d:2572 #3 0x000000000043a256 in std.format.__T13formatGenericTS3std5stdio4File17LockingTextWriterTC4main4testTaZ.formatGeneric() (f=<error reading variable>, arg=0x7fffffffda60, w=...) at /usr/include/d/dmd/phobos/std/format.d:2996 #4 0x000000000043a13a in std.format.__T14formattedWriteTS3std5stdio4File17LockingTextWriterTaTC4main4testZ.formattedWrite() (_param_2=0x7ffff7ed8fe0, fmt=..., w=...) at /usr/include/d/dmd/phobos/std/format.d:506 #5 0x0000000000439c58 in std.stdio.File.__T5writeTC4main4testTaZ.write() ( this=0x695f80 <std.stdio.stdout()>, _param_1=10 '\n', _param_0=0x7ffff7ed8fe0) at /usr/include/d/dmd/phobos/std/stdio.d:744 #6 0x0000000000439baa in std.stdio.__T7writelnTC4main4testZ.writeln() ( _param_0=0x7ffff7ed8fe0) at /usr/include/d/dmd/phobos/std/stdio.d:1746 #7 0x0000000000437c3d in D main () at main.d:24 //file was modified

I don't think this is a bug. Since the array is in a class, it is essentially passed by reference and range algorithms generally mutate ranges. Any class range will be consumed by writeln. I think it would be a bug if the range was not consumed. Implicit conversions make this quite tricky to track though.

Eh, shouldn't it just call t.toString() without even needing to look at alias this? I'm leaning toward either phobos bug or template specialization/constraint change.

(In reply to comment #8) > Eh, shouldn't it just call t.toString() without even needing to look at alias > this? I'm leaning toward either phobos bug or template > specialization/constraint change. Due to the alias this, the class is a range, so it calls the range version of writeln.

(In reply to comment #9) > (In reply to comment #8) > > Eh, shouldn't it just call t.toString() without even needing to look at alias > > this? I'm leaning toward either phobos bug or template > > specialization/constraint change. > > Due to the alias this, the class is a range, so it calls the range version of > writeln. Since a recent compiler enhancement it is easy to extract 'alias this' information from an aggregate. writeln() should do the sane thing and check this before attempting to consume the range. This might be special behavior, but writeln() should IMHO not have side-effects. Here's an approach: ----- static import std.stdio; import std.range; void writeln(T)(T t) { static if (isInputRange!(typeof(__traits(getMember, T, __traits(getAliasThis, T))))) { auto arr = t[]; // slice it std.stdio.writeln(arr); } } class C { uint[] arr; alias arr this; this() { arr = [1, 2]; } } void main() { C c = new C(); writeln(c); writeln(c); } -----

(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > Eh, shouldn't it just call t.toString() without even needing to look at alias > > > this? I'm leaning toward either phobos bug or template > > > specialization/constraint change. > > > > Due to the alias this, the class is a range, so it calls the range version of > > writeln. > > Since a recent compiler enhancement it is easy to extract 'alias this' > information from an aggregate. writeln() should do the sane thing and check > this before attempting to consume the range. This might be special behavior, > but writeln() should IMHO not have side-effects. writeln *must* have side-effects to work with input ranges. How do you consume a one-pass input without modifying it? Alias this isn't the problem. Any reference type range will be consumed. auto a = inputRangeObject([1, 2, 3]); writeln(a); // [1, 2, 3] writeln(a); // [] If you don't want it to be consumed, you need to call .save

(In reply to comment #11) > writeln *must* have side-effects to work with input ranges. How do you consume > a one-pass input without modifying it? Fair enough. It is a bit of a surprise to new users though, but then again 'alias this' is an advanced feature.

Hmm, I've been looking into this more, and it appears the behaviour may never be stable across releases. Consider this: class A { int[] a = [1, 2, 3]; alias a this; } void main() { auto a = new A(); foo(a); } void foo(R)(R r) { writeln(r); } All is fine here. R = A will be deduced, and foo will consume the range as I have explained. Now, suppose a specialisation/overload is added for foo: void foo(int[] r) { writeln(r); } Shouldn't change anything right? It does the same thing as the original foo, just with a more specific parameter type. The problem is now the array is passed in directly by value, which means the range is saved. foo(int[]) is more specialised than foo(R) so the implicit conversion happens and the copy is made. Now, the range will not be consumed. In summary, if you have a class that uses alias this on a value type range then you cannot be sure if the range will be consumed or not: it depends what overloads and specialisations exist :-(

(In reply to comment #13) > Hmm, I've been looking into this more, and it appears the behaviour may never > be stable across releases. > > ... [snip] ... Actually, turns out that behaviour is a bug, which I have now filed. Issue 12117

(In reply to Adam D. Ruppe from comment #8) > Eh, shouldn't it just call t.toString() without even needing to look at > alias this? I'm leaning toward either phobos bug or template > specialization/constraint change. Yes, the bug here is that it sees the item as a range over a more specific "I know how to represent myself as a string". Removing wrong-code, this isn't a compiler issue.

OK, people may not like this, but this is actually expected and defined behavior. From here: https://dlang.org/phobos/std_format.html#.formatValue.10 For the class objects which have input range interface, * If the instance toString has overridden Object.toString, it is used. * Otherwise, the objects are formatted as input range. The example in question *does* have the input range interface, which is defined as follows: is(typeof(R.init) == R) && is(ReturnType!((R r) => r.empty) == bool) && is(typeof((return ref R r) => r.front)) && !is(ReturnType!((R r) => r.front) == void) && is(typeof((R r) => r.popFront)); And it does not define toString. So format is going to defer to the range interface, which in this case, passes the int[] by reference essentially. formatValue was deliberately written this way, so I can't see how it's a bug. (In reply to Peter Alexander from comment #9) > (In reply to comment #8) > > Eh, shouldn't it just call t.toString() without even needing to look at alias > > this? I'm leaning toward either phobos bug or template > > specialization/constraint change. > > Due to the alias this, the class is a range, so it calls the range version > of writeln. In fact, it properly calls the class version of writeln (or formatValue), and this in turn checks for the overloading of toString (at runtime, BTW), and then ends up calling the range version. If you alias this it to any other types and don't override toString, it calls the appropriate format function for that. See the code here: https://github.com/dlang/phobos/blob/ea1e9b2aa3a93780adfff928e8acc629b8db9627/std/format.d#L3448