Eigenstate: myrddin-dev mailing list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: RFC: comparable and hashable traits


Awesome, I want to thank you again for doing this. It took a
lot of work, and exposed an uncomfortable number of bugs and
edge cases in usefiles.

I've pushed to the `hashtraits` branch, since I want a few days
to fix my code, and send around a few fixes to others.

On Wed,  1 Nov 2017 16:53:24 -0300, Lucas Gabriel Vuotto <lvuotto92@xxxxxxxxx> wrote:

> Hello myrddin-dev,
> 
> This is a proposal to change how hash tables are used. The idea is to add two
> new traits to the core language: comparable and hashable. This allows to remove
> the functions that are passed to mkht, removes a lot of duplicated code by just
> providing implementations for @a[:], @a::(integral,numeric) and @a#. As a side
> effect of this, the code becomes cleaner and more readable.

This is a good change. It makes the code more readable, and simplifies the
use of data structures. The big downside is that we lose the ability to do
simple custom hashing, which I've used in a few places that needed case
insensitive key-value pairs:

	var ht = std.mkht(std.strcasehash, std.strcasecmp)

This can be hacked around by using typedefs to change the type being
hashed, though:

	type caseless = byte[:]
	impl hashable caseless = ...
	impl comparable caseless = ...
	var ht = std.mkht()
	std.htput(ht, (str : caseless))

It's more code in rare circumstances, but everything else improves,
so that's a worthwhile trade.

It also opens the doors for a few more changes. I've
been eyeing 'trait std.readable' for a while:

	trait readable @a = ...
	impl std.readable std.fd = ...
	impl std.readable bio.file = ...
	impl std.readable byte[:] = ...

Which would allow generics to abstract over any input source.

> It's important to note that this is a first approach, as there are some places
> where further changes can be made (e.g., replacing sleq with cmp), and some
> things to be done yet (modifying the tests and providing docs). Feedback will
> be appreciated.

Yes, I think that there's room to improve on this, but this lays the
groundwork. I don't think there's anything in this patch set that needs
to be fixed before I can merge to master.

Right now, other than what you mentioned, the big change to the default
functions I'd make would be doing "deep" hashing by default, instead of
a shallow siphash on slices.

The last thing is that the names 'comparable' and 'cmp' makes me think
of ordering rather than simple equality comparison. Maybe calling it
'equatable' and 'eq' would be good? Or maybe this is bikeshedding and
it doesn't matter.

-- 
    Ori Bernstein

Follow-Ups:
Re: RFC: comparable and hashable traitsChris Waldon <christopher.waldon.dev@xxxxxxxxx>
Re: RFC: comparable and hashable traitsOri Bernstein <ori@xxxxxxxxxxxxxx>
References:
RFC: comparable and hashable traitsLucas Gabriel Vuotto <lvuotto92@xxxxxxxxx>