use generic pointers #123
use generic pointers #123
Conversation
It sucks, but, as far as I know, this is the only way to avoid reflection and other pointy nastiness.
| valueType := reflect.TypeOf(t.Value).Elem() | ||
| value := reflect.New(valueType).Interface().(T) | ||
|
|
There was a problem hiding this comment.
Another arguably cleaner option is to add New parametrized method on HamtValue. Then, everywhere you need to instantiate a new value of parametrized typed, you do smth like:
var v V
v = v.New()and keep your type safety without any!
There was a problem hiding this comment.
The next step towards making it even more ergonomic is a generic constructor func:
type HamtValue[V any] interface {
New() V
}
func New[V HamtValue[V]() (v V){
return v.New()
}Then, anywhere you need generic instantiation you simply use the generic New func.
There was a problem hiding this comment.
Hm. You're right. We'd also need an IsNil(), but that works much better.
There was a problem hiding this comment.
Ah, we don't even need that.
There was a problem hiding this comment.
Or IsZero() to be a bit more idiomatic.
There was a problem hiding this comment.
@Wondertan I just used Equal(zeroValue).
Stebalien
force-pushed
the
steb/generic-pointers
branch
from
d55f695
to
b1d76a0
Compare
| type HamtValue[T any] interface { | ||
| Equal(T) bool | ||
| type HamtValue[V any] interface { | ||
| New() V |
There was a problem hiding this comment.
I can't say I love this bit, although it's nicer than seeing **
There was a problem hiding this comment.
I don't love it either, but it beats my previous approach and, as you say, beats **.
| func zero[T any]() T { | ||
| var zero T | ||
| return zero | ||
| } | ||
|
|
||
| // Returns true if the hamt value equals the zero value. | ||
| func isHamtValueZero[T HamtValue[T]](v T) bool { | ||
| return v.Equal(zero[T]()) | ||
| } |
There was a problem hiding this comment.
When using a pointer type, this ends up checking for nil, right? that's what we want for those cases; I'm slightly concerned about valid zero values being the desired case where we are using this to check for "should delete". If you want to insert the zero value in, then you have no way to achieve that now.
I think we might be able to get around some of this awkwardness by refactoring modifyValue because it's currently overloaded for the insert & delete case but the public API has a Delete method so we should strictly apply set and delete semantics to the API and then have the freedom internally to chase that intent down to the bottom.
There was a problem hiding this comment.
When using a pointer type, this ends up checking for nil, right?
Yes.
valid zero values being the desired case where we are using this to check for "should delete".
Yeah..., but that only matters if you want 'nil' to be a valid value.
I think we might be able to get around some of this awkwardness by refactoring modifyValue because it's currently overloaded for the insert & delete case but the public API has a Delete method so we should strictly apply set and delete semantics to the API and then have the freedom internally to chase that intent down to the bottom.
I think this is the way to go, but I'd do this in a followup patch.
There was a problem hiding this comment.
Yeah..., but that only matters if you want 'nil' to be a valid value.
But with the current form here we're not limiting the generic type to be a pointer, it could be a Foo{ id int } in which case the empty value of {id:0} may be what you want to store.
Maybe this can just be clarified with docs though, recommend use of a pointer type.
|
I've pulled this in to |
It sucks, but, as far as I know, this is the only way to avoid reflection and other pointy nastiness.After applying @Wondertan, it's less terrible.