Bug 2451 – Adding structs that use opAssign or postblit to an AA is broken

Status
RESOLVED
Resolution
FIXED
Severity
critical
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
x86
OS
Windows
Creation time
2008-11-13T09:51:00Z
Last change time
2015-06-09T01:20:25Z
Keywords
patch, wrong-code
Assigned to
nobody
Creator
samukha

Comments

Comment #0 by samukha — 2008-11-13T09:51:49Z
import std.variant; void main() { Variant[string] a; Variant v = 1; a["wut?"] = v; } ---- core.exception.ArrayBoundsException@Test(23): Array index out of bounds Key type is irrelevant.
Comment #1 by andrei — 2009-02-22T09:39:45Z
(In reply to comment #0) > import std.variant; > > void main() > { > Variant[string] a; > Variant v = 1; > a["wut?"] = v; > } > ---- > core.exception.ArrayBoundsException@Test(23): Array index out of bounds > > Key type is irrelevant. > I reduced this further: struct Wyda { void opAssign(Wyda) {assert(&this !is null);} } void main() { Wyda[int] a; a[4] = Wyda(); } The assert will fail! Hash tables for value types that define opAssign seem to have a problem.
Comment #2 by dsimcha — 2010-01-15T12:20:57Z
Adding structs that use opAssign or postblit to an AA is broken. The following also produces a range violation: struct Foo { this(this){} } void main() { Foo[string] stuff; stuff["foo"] = Foo.init; }
Comment #3 by andrei — 2010-01-15T13:16:14Z
Perfect timing, thanks. I just ran into that but had no time to investigate. The type Tuple!(uint, "count", float, "distance")[uint] does not work, but the type S[uint] (where struct S { uint count; float distance; }) does work.
Comment #4 by ggcoding — 2010-02-15T12:14:09Z
import std.variant; Variant[char[]][int] aa; aa[0]["a"] = "bla0"; aa[0]["b"] = 100; aa[1]["a"] = "bla1"; aa[1]["b"] = 200; With 32-bit Linux and dmd2.039 or dmd2.040 Compile : success Running : core.exception.RangeError@test(30): Range violation
Comment #5 by clugdbug — 2010-05-22T07:15:10Z
I don't have a patch for this, but the direct reason for the observed behaviour is in expression.c, line 9023. If the type being inserted has an opAssign overload, then it drops out of AssignExp::semantic() immediately and discards the rest of the expression. This isn't a real patch, since it still doesn't call postblit. /* If it is an assignment from a 'foreign' type, * check for operator overloading. */ if (t1->ty == Tstruct) { StructDeclaration *sd = ((TypeStruct *)t1)->sym; if (op == TOKassign) { Expression *e = op_overload(sc); + if (e1->op==TOKindex && + ((IndexExp *)e1)->e1->type->toBasetype()->ty == Taarray) + { + // If it is an AA, the assignment + // should be treated as a function call (Bugzilla 2451) + } + else if (e) return e; }
Comment #6 by clugdbug — 2010-06-14T01:32:05Z
*** Issue 2938 has been marked as a duplicate of this issue. ***
Comment #7 by clugdbug — 2010-06-14T06:46:04Z
*** Issue 4121 has been marked as a duplicate of this issue. ***
Comment #8 by clugdbug — 2010-08-19T07:23:47Z
Bug 3705 (Can't add structs with alias this to an AA) is probably related.
Comment #9 by clugdbug — 2010-10-19T15:07:26Z
Here's a patch. Do the opAssign onto a temporary variable, then blit the temporary into the AA as normal. TEST CASE: struct Foo { int z = 3; void opAssign(Foo x) { z= 2;} } struct Foo2 { int z = 3; this(this){ z = 17; } } void main() { Foo[string] stuff; stuff["foo"] = Foo.init; assert(stuff["foo"].z == 2); Foo2[string] stuff2; Foo2 q; stuff2["dog"] = q; assert(stuff2["dog"].z == 17); } PATCH: expression.c line 8966, AssignExp::semantic. --------------------- /* If it is an assignment from a 'foreign' type, * check for operator overloading. */ if (t1->ty == Tstruct) { StructDeclaration *sd = ((TypeStruct *)t1)->sym; if (op == TOKassign) { Expression *e = op_overload(sc); + if (e && e1->op==TOKindex && + ((IndexExp *)e1)->e1->type->toBasetype()->ty == Taarray) + { + // Deal with AAs (Bugzilla 2451) + // Rewrite as: + // e1 = (typeof(e2) tmp = void, tmp = e2, tmp); + Identifier *id = Lexer::uniqueId("__aatmp"); + VarDeclaration *v = new VarDeclaration(loc, e2->type, + id, new VoidInitializer(NULL)); + v->storage_class |= STCctfe; + Expression *de = new DeclarationExp(loc, v); + VarExp *ve = new VarExp(loc, v); + AssignExp *ae = new AssignExp(loc, ve, e2); + e = ae->op_overload(sc); + e2 = new CommaExp(loc, new CommaExp(loc, de, e), ve); + e2 = e2->semantic(sc); + } + else if (e) return e;
Comment #10 by clugdbug — 2010-10-19T15:13:39Z
*** Issue 1886 has been marked as a duplicate of this issue. ***
Comment #11 by bugzilla — 2010-10-20T00:16:30Z