On Mon, 26 May 2014 19:03:22 -0400, H. S. Teoh via Digitalmars-d
<[email protected]> wrote:
On Sat, May 24, 2014 at 11:54:47PM -0700, Steven Schveighoffer via
Digitalmars-d wrote:
On Sat, 24 May 2014 21:39:14 -0700, H. S. Teoh via Digitalmars-d
<[email protected]> wrote:
>On Sat, May 24, 2014 at 06:05:49PM -0700, Steven Schveighoffer via
>Digitalmars-d wrote:
>>On Sat, 24 May 2014 02:54:01 -0700, FG <[email protected]> wrote:
>[...]
>>>Really? Then what does TypeInfo.compare(void*, void*) use? For
>>>example here:
>>>
>>> auto key_hash = keyti.getHash(pkey); Entry *e;
>>> /* ... */
>>> if (key_hash == e.hash) {
>>> auto c = keyti.compare(pkey, e + 1);
>>> if (c == 0) goto Lret;
>>> }
>>
>>You know what, you are right. I assumed it used keyti.equals. This
>>is a bug imo, since opCmp will be used, and opEquals will be
>>ignored. Just checking for opCmp == 0 is identical to opEquals,
>>except some types can define opEquals but not opCmp.
>>
>>But I don't know if it will get fixed. The whole AA runtime has to
>>be redone at some point.
>[...]
>
>This has been argued over in a druntime pull before. I'm 100% for
>changing the above line (and all other instances of it) to use
>keyti.equals() instead. But that was shot down due to potential
>breakage of existing code. :-( :-( Nevermind the fact that it
>probably breaks a lot more *new* code than it ever will break old
>code... :-(
Any object/struct that defines opCmp but not opEquals is broken, and
deserves to not work with AAs.
It's a trivial change to add opEquals when opCmp is defined. This change
should happen should happen. What pull request is it?
Sorry for the late response, I've been very busy with other things.
I can't seem to find the PR anymore (it got closed over disagreement
about how things should be fixed), but I did find the branch my code was
in. The bugzilla issue is 10380. Looking at the bug, I see that
apparently Denis has pushed through a hack fix for it:
https://github.com/D-Programming-Language/druntime/pull/736
Hah, looking at that PR, it references the original PR
(https://github.com/D-Programming-Language/druntime/pull/522). In which I
commented, I remember this one. And look, there are the rules I outlined :)
But I think there was some indication that a runtime-based version was
imminent. I don't know what happened to that.
Going to comment again on that PR. We should do something to make our way
back to sane logic.
>It's been far too long that AA's have been broken in druntime.
>Somebody should just take the aaA.d out in the back and shoot it, and
>replace it with a better implementation. I think users will be far
>happier with that.
I don't know that the current implementation is broken, just horrible.
I'd rather focus on getting a full library type able to be up-to-par
with the builtin type, and then switch the implementation over in the
compiler. IIRC, the compiler does some magic (i.e. ignoring certain
attribute requirements) that can't be done for a library type.
[...]
It's not that hard to get a library AA to be on par with the builtin
type. The last time I tried it, though, I got stuck on trying that solve
const issues that even the builtin type doesn't solve correctly. The
main showstopper, however, is the amount of effort it takes to extricate
the current AA implementation from the compiler. There are just too many
places where it relies on compiler magic.
Yes, it would be worth a lot to see if we can create an equivalent AA type
in the library first, BEFORE trying to replace the builtin AA. The way it
was done originally, was more of a mess than the fully compiler-defined AA.
But this doesn't mean we can't fix the runtime.
-Steve
