Bug 11343 – [2.064 beta] Error: multiple field initialization
Status
RESOLVED
Resolution
INVALID
Severity
regression
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-10-24T12:56:00Z
Last change time
2013-10-30T16:49:40Z
Assigned to
nobody
Creator
rswhite4
Comments
Comment #0 by rswhite4 — 2013-10-24T12:56:06Z
Code:
----
import std.stdio;
enum Foo {
A = 1,
B = 2
}
struct Test {
public:
const ubyte[Foo] test;
this(Foo f) {
if (Foo.A & f)
this.test[Foo.A] = 1;
if (Foo.B & f)
this.test[Foo.B] = 2;
}
}
void main() {
Test t = Test(Foo.A);
}
----
Flag_TEst.d(16): Error: multiple field test initialization
Tested with the latest beta.
Worked in 2.063.2
Comment #1 by rswhite4 — 2013-10-24T14:02:05Z
Consider also this:
----
import std.stdio;
struct Test {
public:
const int[] test;
this(int i) {
this.test ~= i;
this.test ~= i + 1;
}
}
void main() {
Test t = Test(42);
}
----
Same error
And maybe related: http://forum.dlang.org/thread/[email protected]%2Fissues%2F
Comment #2 by k.hara.pg — 2013-10-24T17:18:06Z
(In reply to comment #0)
> Code:
> ----
> import std.stdio;
>
> enum Foo {
> A = 1,
> B = 2
> }
>
> struct Test {
> public:
> const ubyte[Foo] test;
>
> this(Foo f) {
> if (Foo.A & f)
> this.test[Foo.A] = 1;
> if (Foo.B & f)
> this.test[Foo.B] = 2;
> }
> }
>
> void main() {
> Test t = Test(Foo.A);
> }
> ----
>
> Flag_TEst.d(16): Error: multiple field test initialization
>
> Tested with the latest beta.
> Worked in 2.063.2
It's intended behavior change introduced by fixing bug 9665.
Non-mutable field initialization is now allowed just only once.
So, in this case, you need to create a temporary mutable AA, then initialize Test.test by that.
struct Test {
public:
const ubyte[Foo] test;
this(Foo f) {
ubyte[Foo] tmp;
if (Foo.A & f)
tmp[Foo.A] = 1;
if (Foo.B & f)
tmp[Foo.B] = 2;
this.test = tmp;
}
}
Comment #3 by k.hara.pg — 2013-10-24T17:22:40Z
(In reply to comment #1)
> Consider also this:
>
> ----
> import std.stdio;
>
> struct Test {
> public:
> const int[] test;
>
> this(int i) {
> this.test ~= i;
> this.test ~= i + 1;
> }
> }
>
> void main() {
> Test t = Test(42);
> }
> ----
>
> Same error
The code modifies a non-mutable array field 'const(int[]) test' twice.
Create temporary int[] and initialize Test.test by that.
Comment #4 by rswhite4 — 2013-10-25T00:01:24Z
After my imagination, a constant member can be assigned as many times as I want, as long as it happens in the constructor. This is only logical. Everything else just makes it complex and completely non intuitive, especially for newbies.
Comment #5 by k.hara.pg — 2013-10-25T00:52:00Z
(In reply to comment #4)
> After my imagination, a constant member can be assigned as many times as I
> want, as long as it happens in the constructor. This is only logical.
> Everything else just makes it complex and completely non intuitive, especially
> for newbies.
A const reference may refer immutable data which comes from the out of the constructor. If so, modifying const data inside constructor is definitely illegal operation. By disallowing multiple initialization, compiler can validate the field initializing correctness or not, at the first initializing place.
====
Additionally, it is necessary to make immutable field data initialization more flexible.
Today, if a pure function returns mutable data, it could be implicitly convertible to immutable data.
int[] make(int n) pure {
int[] a = new int[](n);
foreach (i; 0 .. n)
a[i] = i + 1;
return a; // [1, 2, ..., n]
}
immutable int[] arr = make(n); // OK
From 2.064, by fixing issue 9665, compiler can detect the only once initializing point for the object fields.
Combine pure function, you can initialize immutable data field as follows.
struct S
{
immutable int[] arr;
this(int n)
{
static int[] make(int n) pure {
int[] a = new int[](n);
foreach (i; 0 .. n)
a[i] = i + 1;
return a; // [1, 2, ..., n]
}
arr = make(n);
// This line is now initialization (not assignment), so
// conversion from mutable to immutable is allowed
}
}
void main()
{
auto s = S(10);
assert(s.arr[0] == 0);
assert(s.arr[$-1] == 10);
static assert(is(typeof(s.arr) == immutable int[]));
}
Comment #6 by k.hara.pg — 2013-10-25T02:08:50Z
Mark as invalid, as same as bug 11346.
Comment #7 by bearophile_hugs — 2013-10-28T15:19:49Z
(In reply to comment #5)
> A const reference may refer immutable data which comes from the out of the
> constructor. If so, modifying const data inside constructor is definitely
> illegal operation. By disallowing multiple initialization, compiler can
> validate the field initializing correctness or not, at the first initializing
> place.
To reduce a little the disruption of formerly compilable code is it possible and a good idea to make the detector smarter, and allow code like this?
struct Foo {
immutable int[3] arr;
this(int x) {
arr[0] = x;
arr[1] = x + 1;
arr[2] = x + 2;
}
}
void main() {
auto f = Foo(5);
}
Currently it gives:
test.d(5): Error: multiple field arr initialization
test.d(6): Error: multiple field arr initialization
The idea is to see and track every cell of a fixed-size array as an independent variable, like this, that compiles:
struct Foo {
immutable int arr0, arr1, arr2;
this(int x) {
arr0 = x;
arr1 = x + 1;
arr2 = x + 2;
}
}
void main() {
auto f = Foo(5);
}
If this is possible and good, then I could open an enhancement request on this.
(Once that's possible, in future we could even allow some simple forms of initialization in loops, but that's for later).
Comment #8 by k.hara.pg — 2013-10-29T18:51:45Z
(In reply to comment #7)
> To reduce a little the disruption of formerly compilable code is it possible
> and a good idea to make the detector smarter, and allow code like this?
>
> struct Foo {
> immutable int[3] arr;
> this(int x) {
> arr[0] = x;
> arr[1] = x + 1;
> arr[2] = x + 2;
> }
> }
I think this is difficult if one of the index is given in runtime. For example:
this(int x, size_t i) {
arr[0] = x;
arr[1] = x + 1;
arr[i] = x + 2; // which index will be initialized in runtime?
}
Once arr[i] is used inside constructor, compiler should reject *all other* initializing of arr (arr[0]=x; and arr[1]=x+1;) to avoid multiple initialization. I think there's not benefits commensurate with the cost to implement it. So I won't work for that.
Instead, I'd like to recommend an alternative way how to initialize immutable int[3] field. Today, NRVO also works for static array data, and you can convert mutable data to immutable implicitly by using pure *creator* function.
this(int x) {
static int[3] make(int x) pure {
int[3] a;
a[0] = x;
a[1] = x + 1;
a[2] = x + 2;
}
this.arr = make(x); // NRVO works
}
In near the future, you will be able to use a pure lambda instead of the static 'make' function.
Comment #9 by rswhite4 — 2013-10-30T01:24:58Z
This is annoying. If I want to initialize an const array I must write a new function? No way. The easier way (and I'm sure the most users will do this) is to get rid of const. That change may guarantee more safety but it will also lead that const is used less.
Comment #10 by rswhite4 — 2013-10-30T01:44:17Z
Couldn't the compiler to this dirty trick for me?
If he found such multiple init case, he could rewrite the code. So that this code:
----
this(int x, size_t i) {
arr[0] = x;
arr[1] = x + 1;
arr[i] = x + 2; // which index will be initialized in runtime?
}
----
if rewritten to this:
this(int x, size_t i) {
Unqual!typeof(arr) __arr = null;
__arr[0] = x;
__arr[1] = x + 1;
__arr[i] = x + 2;
this.arr = __arr; /// only _one_ assignment
}
Comment #11 by k.hara.pg — 2013-10-30T01:49:54Z
(In reply to comment #10)
> Couldn't the compiler to this dirty trick for me?
> If he found such multiple init case, he could rewrite the code. So that this
> code:
> ----
> this(int x, size_t i) {
> arr[0] = x;
> arr[1] = x + 1;
> arr[i] = x + 2; // which index will be initialized in runtime?
> }
> ----
>
> if rewritten to this:
>
> this(int x, size_t i) {
> Unqual!typeof(arr) __arr = null;
>
> __arr[0] = x;
> __arr[1] = x + 1;
> __arr[i] = x + 2;
>
> this.arr = __arr; /// only _one_ assignment
> }
That would be a bad behavior when the size of arr field is quite big, because it will silently consume stack memory. I can believe someone will hate it.
Comment #12 by rswhite4 — 2013-10-30T01:52:32Z
(In reply to comment #11)
> (In reply to comment #10)
> > Couldn't the compiler to this dirty trick for me?
> > If he found such multiple init case, he could rewrite the code. So that this
> > code:
> > ----
> > this(int x, size_t i) {
> > arr[0] = x;
> > arr[1] = x + 1;
> > arr[i] = x + 2; // which index will be initialized in runtime?
> > }
> > ----
> >
> > if rewritten to this:
> >
> > this(int x, size_t i) {
> > Unqual!typeof(arr) __arr = null;
> >
> > __arr[0] = x;
> > __arr[1] = x + 1;
> > __arr[i] = x + 2;
> >
> > this.arr = __arr; /// only _one_ assignment
> > }
>
> That would be a bad behavior when the size of arr field is quite big, because
> it will silently consume stack memory. I can believe someone will hate it.
Who?
It's the best approach I have currently. There must be a way to weaken the effects of this change.
Comment #13 by bearophile_hugs — 2013-10-30T02:43:17Z
(In reply to comment #8)
> this(int x) {
> static int[3] make(int x) pure {
> int[3] a;
> a[0] = x;
> a[1] = x + 1;
> a[2] = x + 2;
> }
> this.arr = make(x); // NRVO works
> }
A little test program:
struct Foo {
int[3] arr;
this(int x) pure nothrow {
arr[0] = x;
arr[1] = x + 1;
arr[2] = x + 2;
}
}
struct Bar {
immutable int[3] arr;
this(int x) pure nothrow {
static int[3] make(in int x) pure nothrow {
typeof(return) a;
a[0] = x;
a[1] = x + 1;
a[2] = x + 2;
return a;
}
this.arr = make(x); // NRVO works
}
}
void main() {
auto f = Foo(5);
auto b = Bar(5);
}
The asm of its struct constructors using dmd:
dmd -O -release -inline -noboundscheck test.d
Foo.__ctor:
push EBX
mov EDX, EAX
mov EBX, 8[ESP]
push ESI
lea ECX, 1[EBX]
lea ESI, 2[EBX]
mov [EDX], EBX
mov 4[EDX], ECX
mov 8[EDX], ESI
pop ESI
pop EBX
ret 4
Bar.__ctor:
sub ESP, 010h
xor EDX, EDX
push EBX
mov EBX, 018h[ESP]
push ESI
lea ESI, 1[EBX]
push EDI
lea EDI, 2[EBX]
mov 018h[ESP], EAX
push 0Ch
lea ECX, 010h[ESP]
mov [ECX], EDX
mov 4[ECX], EDX
mov 8[ECX], EDX
mov 010h[ESP], EBX
mov 014h[ESP], ESI
mov 018h[ESP], EDI
push ECX
push EAX
call near ptr _memcpy
add ESP, 0Ch
mov EAX, 018h[ESP]
pop EDI
pop ESI
pop EBX
add ESP, 010h
ret 4
Comment #14 by k.hara.pg — 2013-10-30T03:00:47Z
(In reply to comment #12)
> Who?
Embedded system programmers, for example.
---
We can add some acceptable cases certainly. For example, by the following rule:
"If the static array field is initialized
1. with different compile-time constant indices, and
2. each initializing expressions does not have overlapping
with each other over the whole control-flow paths
in the constructor"
the bearophile's case may be allowed.
But, it's very complex and heuristic rule, and will covers just only a few cases.
We must reject heuristic rule, because it is hard to maintain and verify language semantics.
That's the matter of cost-benefit ratio.
Comment #15 by k.hara.pg — 2013-10-30T03:12:27Z
(In reply to comment #13)
> (In reply to comment #8)
>
> A little test program:
[snip]
Hmm, strange. After moving out the nested static function 'make' to module level, I got this generated code.
struct Foo {
int[3] arr;
this(int x) pure nothrow {
arr[0] = x;
arr[1] = x + 1;
arr[2] = x + 2;
}
}
// move out to module level
static int[3] make(in int x) pure nothrow {
typeof(return) a;
a[0] = x;
a[1] = x + 1;
a[2] = x + 2;
return a;
}
struct Bar {
immutable int[3] arr;
this(int x) pure nothrow {
this.arr = make(x); // NRVO works?
}
}
void main() {
auto f = Foo(5);
auto b = Bar(5);
}
Assembly:
C:\Users\khara\d\test.d:23 void main() {
0040209c: 83ec24 sub esp, 0x24
0040209f: 31c9 xor ecx, ecx
004020a1: ba05000000 mov edx, 0x5
004020a6: 53 push ebx
004020a7: bb06000000 mov ebx, 0x6
004020ac: 56 push esi
C:\Users\khara\d\test.d:24 auto f = Foo(5);
004020ad: 8d442408 lea eax, [esp+0x8]
004020b1: be07000000 mov esi, 0x7
004020b6: 8908 mov [eax], ecx
004020b8: 894804 mov [eax+0x4], ecx
004020bb: 894808 mov [eax+0x8], ecx
C:\Users\khara\d\test.d:4 arr[0] = x;
004020be: 8d442414 lea eax, [esp+0x14]
004020c2: 89542408 mov [esp+0x8], edx
C:\Users\khara\d\test.d:5 arr[1] = x + 1;
004020c6: 31d2 xor edx, edx
004020c8: 895c240c mov [esp+0xc], ebx
C:\Users\khara\d\test.d:6 arr[2] = x + 2;
004020cc: 89742410 mov [esp+0x10], esi
C:\Users\khara\d\test.d:25 auto b = Bar(5);
004020d0: 6a0c push 0xc
C:\Users\khara\d\test.d:20 this.arr = make(x); // NRVO works?
004020d2: 8908 mov [eax], ecx
004020d4: 894804 mov [eax+0x4], ecx
004020d7: 894808 mov [eax+0x8], ecx
004020da: 8d4c2424 lea ecx, [esp+0x24]
004020de: 8911 mov [ecx], edx
004020e0: 895104 mov [ecx+0x4], edx
004020e3: 895108 mov [ecx+0x8], edx
C:\Users\khara\d\test.d:11 a[0] = x;
004020e6: c744242405000000 mov dword [esp+0x24], 0x5
C:\Users\khara\d\test.d:12 a[1] = x + 1;
004020ee: 895c2428 mov [esp+0x28], ebx
C:\Users\khara\d\test.d:13 a[2] = x + 2;
004020f2: 8974242c mov [esp+0x2c], esi
C:\Users\khara\d\test.d:10 typeof(return) a;
004020f6: 51 push ecx
004020f7: 50 push eax
004020f8: e8e3110100 call 0x4132e0 _memmove // still exists...!
004020fd: 83c40c add esp, 0xc
C:\Users\khara\d\test.d:26 }
00402100: 31c0 xor eax, eax
00402102: 5e pop esi
00402103: 5b pop ebx
00402104: 83c424 add esp, 0x24
00402107: c3 ret
In 2.064a, the bug 10094 has been fixed. However we need to fix these (at least two orthogonal) bugs for the next release.
I'll open new issues for them.
Comment #16 by bearophile_hugs — 2013-10-30T16:49:40Z
(In reply to comment #15)
> In 2.064a, the bug 10094 has been fixed. However we need to fix these (at least
> two orthogonal) bugs for the next release.
>
> I'll open new issues for them.
Something perhaps related:
import std.algorithm: copy;
immutable int[2] data;
static this() {
[10, 20].copy(data[]); // Error.
}
void main() {}
Thread:
http://forum.dlang.org/thread/[email protected]