-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Look into Hash / PartialEq implemented on Value #15
Comments
Some rectifications here: In other words, if Some kind of solution:I think the best way to implement this is to slightly modify how As far as I know, special forms are empty structures, so we would just have to add Regular forms (represented by Now we need to tell the compiler that every kind of structure which implements pub trait IFn: Debug + DynClone + Hash + Eq {
// ...
} Note:Please do not automatically derive Note (2):It may be noted that I am a Rustacean who like LISPs, but never used them. As such, the implementation described in the paragraph before may be entirely wrong. I am making the assumption that two functions declared at two different places in the code, even if they arguments and bodies are the same, won't result in the same hash. Please correct me if I'm wrong. Edit: fix typo. |
I appreciate this, but you don't have to explain what a partial equivalence is, or the relationship between [derive(...PartialEq,Eq,Hash) , so I need to implement them manually as impl PartialEq for Value {
....
}
impl Eq for Value {}
impl Hash for Value {
...
} The thing is, as we know, if impl Hash for Value {
... Still defining this
}
// I'm just pulling this example function straight from the std::Hash docs
fn calculate_hash<T: Hash>(t: &T) -> u64 {
let mut s = DefaultHasher::new();
t.hash(&mut s);
s.finish()
impl PartialEq for Value {
fn eq(&self, other: &Value) -> bool {
// Here we just reuse our hashes
calculate_hash(self) == calculate_hash(other)
}
}
impl Eq for Value {} Another reason I was specifically thinking this is I believe Clojure proper works this way as well, although I have to check again; that it uses an efficient hashing algorithm on objects, and uses that directly for checking equality. And indeed, two functions with the same definition are not equal / do not hash the same.
pub trait IFn: Debug + DynClone + Hash + Eq {
// ...
} Again, unless I'm missing something, give me more credit ahaha (EDIT: not literally, I'm glad to hear suggestions. But I likewise wanted to do this rather than resorting to what's there now). You can't do this outright because |
TLDR: my previous comment was very wrong. I failed to understand what you meant by implementing equality thanks to hash. Your idea is valid. You're also right about trait object safety. Trait objects are things I try to avoid in my own code. As such, their rules are not always clear to me. It may also be useful to quote my apologies from the Discord server:
I am quite sure that the Hashing algorithm used in the Rust standard library is not designed to be fast, but rather to generate at least hash collisions as possible. So this may decrease performances, which is fine since we are in early phases of development. I think that it's the only thing I can add to the conversation right now. I definitely lack some Clojure knowledge. |
First of all I just want to say that maybe I'm missing something as I have never written Nevertheless if I have understood correctly, could implementing Perhaps |
I've thought about that but I don't know yet. I suspect so, and its a case where I wanted to also look at the clojure base and see if that sort of thing's a problem, and how its handled, or see if I was mistaken about just how hashes are used there. I did forget about this when I wrote this issue, though, as I forgot I was considering dropping this idea altogether because of this. Regardless, I want to look into it |
If I recall, due to
Value
containingAs well as some similar cases containing trait objects, we can not just use
[derive(..PartialEq,Hash)]
, but we do need these as our persistent maps will use Values as their keys.Since a rough, basic persistent map was added at the last minute, so was a last minute implementation of
Hash
andPartialEq
on Value. However, I'm pretty sure only Hash needs to be defined, and PartialEq can be defined in terms of Hash. Whatever the case, this code is an example of something that was something of a last minute hack, and has not been proofread yet, so I'd like to make sure this implemented as it should be.The text was updated successfully, but these errors were encountered: