Bug 4410 – AA has inconsistent and unreasonable requirements for iterating over reference-type index

Status
RESOLVED
Resolution
WORKSFORME
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
Other
OS
All
Creation time
2010-07-01T05:46:43Z
Last change time
2022-08-16T14:30:04Z
Assigned to
No Owner
Creator
Steven Schveighoffer

Comments

Comment #0 by schveiguy — 2010-07-01T05:46:43Z
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.
Comment #1 by schveiguy — 2010-07-01T05:53:32Z
(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.
Comment #2 by nfxjfg — 2010-07-01T18:58:51Z
Obviously, AA keys must be immutable.
Comment #3 by schveiguy — 2010-07-02T05:15:40Z
(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.
Comment #4 by hsteoh — 2013-08-15T09:56:29Z
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.
Comment #5 by razvan.nitu1305 — 2022-08-15T11:38:08Z
Apart from the first foreach, the rest do not compile today. You can only ref an aa key if it is const. This makes sense in my opinion. @Steven, what else needs to be done to consider this issue fixed?
Comment #6 by schveiguy — 2022-08-16T14:30:04Z
I wrote this so so long ago. I'm not sure this bug is relevant any more. Problems still do exist with keys being not-constant for reference types (classes), but iteration is not the problem there.