Feedback Thread: DIP 1042--ProtoObject--Community Review Round 1

[Dukc]

On Monday, 10 January 2022 at 13:48:42 UTC, Mike Parker wrote:
> ## Feedback Thread
>
> This is the feedback thread for the first round of Community 
> Review of DIP 1042, "ProtoObject".

I still think that requiring comparisons and `toHash` functions 
to be `@nogc` is too strict. It is true that an ideally they 
should always be such, but the new interfaces should also be 
usable by not-so-successful code. It's likely the users will just 
continue to use the old object. Or worse, they will hack their 
functions to be `@nogc` with `@trusted` `malloc`s or `alloca`s. 
Classes are not used much in `@nogc` scenarios anyway, so it's 
not a big loss that `hashableObject.toHash()` does not guarantee 
no gc.

I do think `pure` and `@safe` are justified in the new 
interfaces. Those are attributes that the large majority of code 
already has or can easily have. For `nothrow`, not sure. OTOH it 
isn't a breaking change and can be worked around with 
`try{/*former implementation*/} catch (Exception) assert(0)`, 
OTOH there is still risk of continued usage of old `Object` or 
using the mentioned workaround carelessly.

What does `interface Stringify` look like? Didn't spot that in 
the DIP.

The `_cmp` function immeditely returns -1 if the first argument 
is not `Ordered`. Why? I'd expect it to first try casting the 
second argument to `Ordered` and use it's `cmp` against the first 
argument.

The Implement* template mixins are not worth including in this 
DIP. The users aren't worse off without them that with the 
present `Object` default implementations - it's dead easy to ape 
them. You might want to include simple functions that do what 
`Object` does (but no function for `cmp` please - `Object` just 
throws an exception), but that's it. I'm not saying the template 
mixins are a bad idea, just that they are orthogonal to what this 
DIP proposes.

> Implementations of `Ordered`, `Equals`, and `Hash` must agree 
> with each other. That is, `a.cmp(b) == 0` if and only if `(a == 
> b) && (a.toHash == b.toHash)`. It's easy to accidentally make 
> them disagree by mixing in some of the interface 
> implementations and manually implementing others.

I agree with the "only if" but not with the "if" part here. Hash 
is only 32 bits long on 32-bit platforms, meaning a hash 
collision with only 64000 different values if using a perfectly 
chaotic hash function. For chaotic hashes, strictly no hash 
collisions is impossible on 32 bits and probably way too 
difficult in practice even on 64 bits.