D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 8681 - dmd accepts mutable AA key types for objects
Summary: dmd accepts mutable AA key types for objects
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 major
Assignee: No Owner
URL:
Keywords: accepts-invalid
Depends on:
Blocks:
 
Reported: 2012-09-17 13:29 UTC by Jonathan M Davis
Modified: 2024-12-13 18:01 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Jonathan M Davis 2012-09-17 13:29:18 UTC
I would have thought that this would have been reported already, but I can't find it, so I'm reporting it. AA keys are supposed to be required to be immutable, but the compiler is not correctly enforcing that. For instance, take this code:

void main()
{
    class C
    {
        int id = 42;
    }

    C c = new C;
    int[C] cArr;

    cArr[new C] = 42; //should not compile. new C is not immutable.
    cArr[c] = 23; //should not compile. c is not immutable.

    C value = cArr.keys[0]; //should not compile. keys[0] should be immutable.
    ++value.id;

    struct S
    {
        int id = 12;
    }

    S* s = new S;
    int[S*] sArr;

    sArr[new S] = 7; //should not compile. new S is not immutable.
    sArr[s] = 3; //should not compile. s is not immutable.

    int[char[]] strArr;
    char[] str = "g".dup;
    strArr[str] = 7; //Correctly fails to compile.
}


Every single insertion into AA in that could should fail to compile, but only the char[] one does. Clearly, dmd is not properly checking for immutability with objects. It may be that the AA implementation rewrite which is currently being worked on will need to be completed for this to be properly fixed, but the longer that it stays like this, the more code that will break once it's fixed. And it _needs_ to be fixed, because otherwise there are going to be all kinds of problems with AA keys being changed after they've been used to insert values into an AA.
Comment 1 hsteoh 2013-07-08 20:10:39 UTC
There's a case where accepting mutable keys may be OK:

class C {
    immutable int x;
    int y, z;

    this() { ... /* initialize x */ }

    size_t toHash() const {
        // The "real" data is in x; y and z are used for, e.g., caching
        // or holding temporary values
        return typeid(int).getHash(x);
    }
    bool opEquals(const C c) const {
        return x == c.x;
    }
}

Since both toHash and opEquals only depend on C.x, it should be permissible to accept mutable C as AA key; the immutability of C.x guarantees we won't run into problems.

The problem is, I don't think such a case is expressible in the current type system.

(Though arguably, such cases are probably bad design and shouldn't be encouraged/catered to.)
Comment 2 Marco Leise 2016-09-21 16:15:13 UTC
(In reply to hsteoh from comment #1)
What works with the type system is to isolate the fields from C that define its equality:

struct ImmutableCPart
{
    int x;

    size_t toHash() const {
        return typeid(int).getHash(x);
    }
    bool opEquals(const C c) const {
        return x == c.x;
    }
}

class C {
    immutable ImmutableCPart x;
    int y, z;

    this() { ... /* initialize x */ }
}

C[ImmutableCPart] lookup;
Comment 3 dlangBugzillaToGithub 2024-12-13 18:01:27 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/dmd/issues/18471

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB