Thread overview
[Issue 4410] New: AA has inconsistent and unreasonable requirements for iterating over reference-type index
Jul 02, 2010
nfxjfg@gmail.com
July 01, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=4410

           Summary: AA has inconsistent and unreasonable requirements for
                    iterating over reference-type index
           Product: D
           Version: D2
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: schveiguy@yahoo.com


--- Comment #0 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-07-01 05:46:43 PDT ---
Given the following:
class C {}
struct S {int u;}

uint[C] aa1;
uint[S*] aa2;

We have many issues with foreach:

foreach(C k, v; aa1) {} // compiles
foreach(S *k, v; aa2) {} // does not compile, k must be const(S)*
foreach(ref C k, v; aa1) {} // compiles(!)
foreach(ref const(S) *k, v; aa2) {} // compiles

Here are the issues:

first, ref should *never* be allowed as an index for AAs.  I hope that someday this can be extended to all user types (see related bug 2443).

Second, we have a strange restriction for pointer key types in that the key must be designated tail-const.  I sort of understand this in that you don't want the key to be changing after adding, but this restriction is impossible to enforce -- one can easily change the original pointer passed in.  i.e.:

auto x = new S;
aa2[x] = 5;
x.u = 3; // oops!

In addition, the same restriction isn't required for classes, so you *can* change the class data during iteration.  Finally, even though the pointer is tail-const, allowing a reference to it means you can change the pointer itslef!

I think the const restriction should be removed, it's not enforceable and makes generic code much more difficult, since it's inconsistent.  In addition, we can amend the documentation for AAs to indicate that if you want sane AAs, you should use immutable keys.  Otherwise, it's on you never to change the keys after adding them.

Also, as mentioned, ref indexes should not be allowed.  This used to be the case, not sure why it was changed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 01, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=4410



--- Comment #1 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-07-01 05:53:32 PDT ---
(In reply to comment #0)
> first, ref should *never* be allowed as an index for AAs.  I hope that someday this can be extended to all user types (see related bug 2443).

...

> Also, as mentioned, ref indexes should not be allowed.  This used to be the case, not sure why it was changed.

I should clarify, I mean ref indexes during foreach.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 02, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=4410


nfxjfg@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nfxjfg@gmail.com


--- Comment #2 from nfxjfg@gmail.com 2010-07-01 18:58:51 PDT ---
Obviously, AA keys must be immutable.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 02, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=4410



--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-07-02 05:15:40 PDT ---
(In reply to comment #2)
> Obviously, AA keys must be immutable.

Making all array keys immutable is pretty restrictive as well.  I don't want to see that happening either.  I don't know if you've tried to work with immutable classes, but they are not that easy.

A recommendation that they be immutable when possible is probably in order. But making them const is worse than immutable because it provides no guarantees and is still as annoying.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
August 15, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=4410


hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx


--- Comment #4 from hsteoh@quickfur.ath.cx 2013-08-15 09:56:29 PDT ---
AA keys only need to be immutable in the parts that are relevant to the hash function. For example, if a class object's toHash method simply casts 'this' to an integer value, then it doesn't matter what you change in the class, the AA will still work. Similarly, if a class object's toHash method computes a hash only on members x, y, z, then it's OK to change member variable w without breaking the AA.

The only time you actually require immutability in AA keys is when you use the default hash function that computes a hash over the binary representation of the entire key.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------