October 18
https://issues.dlang.org/show_bug.cgi?id=24820

          Issue ID: 24820
           Summary: Associative arrays do not correctly handle keys with
                    copy constructors
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: critical
          Priority: P1
         Component: druntime
          Assignee: nobody@puremagic.com
          Reporter: issues.dlang@jmdavisProg.com

This code

```
import std.stdio;

void main()
{
    auto count = new Count;

    string[S] aa;
    aa[S(count, 42)] = "foo";

    writeln("---");
    writeln(*count);
    assert(count.refCount == 1);

    writeln("---");
    assert(S(count, 42) in aa);
    writeln("---");
    writeln(*count);
    assert(count.refCount == 1);

    writeln("---");
    assert(S(new Count, 22) !in aa);
    writeln("---");
    writeln(*count);
    assert(count.refCount == 1);
}

struct Count
{
    int constructed;
    int copyConstructed;
    int assignedFrom;
    int assignedTo;
    int destroyed;
    int refCount;

    string toString()
    {
        import std.format : format;

        return format("Constructed: %s times\n", constructed) ~
               format("Copy Constructed: %s times\n", copyConstructed) ~
               format("Assigned From: %s times\n", assignedFrom) ~
               format("Assigned To: %s times\n", assignedTo) ~
               format("Destroyed: %s times\n", destroyed) ~
               format("refCount: %s", refCount);
    }
}

struct S
{
    Count* count;
    int i;
    bool destroyed;

    this(Count* count, int i)
    {
        this.count = count;
        this.i = i;
        ++count.constructed;
        ++count.refCount;
        writefln("Constructing: %s, count(%s)", i, count);
    }

    this(ref S rhs)
    {
        this.count = rhs.count;
        this.i = rhs.i;

        ++count.copyConstructed;
        ++count.refCount;
        writefln("Copy Constructing: %s, count(%s)", i, count);
    }

    ~this()
    {
        writefln("Destroying: %s, count(%s)", i, count);

        if(count)
        {
            ++count.destroyed;
            --count.refCount;
        }

        destroyed = true;
    }

    void opAssign()(auto ref S rhs)
    {
        writefln("Assigning %s, count(%s) to %s, count(%s)",
                 rhs.i, rhs.count, this.i, this.count);

        if(this.count)
        {
            ++this.count.assignedTo;
            --this.count.refCount;
        }

        if(rhs.count)
        {
            ++this.count.assignedFrom;
            ++this.count.refCount;
        }

        this.count = rhs.count;
        this.i = rhs.i;
    }

    invariant
    {
        assert(!destroyed);
    }

    bool opEquals()(const auto ref S rhs) const {
        return this.i == rhs.i;
    }

    size_t toHash() const {
        return 0;
    }
}
```

has this output:

```
Constructing: 42, count(7F7AF01A9000)
Destroying: 42, count(7F7AF01A9000)
---
Constructed: 1 times
Copy Constructed: 0 times
Assigned From: 0 times
Assigned To: 0 times
Destroyed: 1 times
refCount: 0
core.exception.AssertError@q.d(12): Assertion failure
----------------
??:? _d_assertp [0x5e7bf2fe9c74]
??:? _Dmain [0x5e7bf2fc99ca]
Destroying: 42, count(7F7AF01A9000)
```

Pretty clearly, the issue is this line:

    aa[S(count, 42)] = "foo";

Ideally, the temporary would just be moved, and no copies would be necessary, but the AA does need to have a copy of the key so that it can compare against it when doing look-ups later. So, depending on the implementation, it would make sense if the temporary were copied at some point in the process rather than moved, and then the temporary would need to be destroyed. But that means that one of two things should be happening here. Either, the temporary should be moved, and there should be no destructor calls, or the temporary should be copied, resulting in a copy constructor call to get the copy that the AA stores and then a destructor call for the temporary. However, it looks like a copy is probably being made via blitting, and then the temporary is destroyed, which is obviously going to result in the wrong behavior in any case where a copy constructor is actually needed.

Changing the copy constructor to a postblit constructor fixes the problem, so it would appear that AAs simply weren't updated to take copy constructors into account.

And commenting out the assertions for the reference count (so that the code which is checking what happens with the in operator can run) clearly indicates that the in operator isn't handling copy constructors properly either.

--