-
Notifications
You must be signed in to change notification settings - Fork 356
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
Implement Eq
, Clone
and Hash
for MemoryData and Evaluator
#387
Conversation
In order to implement infinite loop detection while executing MIR, both the implementor of `Machine` (`Evaluator`) and its associated type (`MemoryData`), must implement `Eq` and `Hash`. This PR adds the required trait implementations. It's possible that the `Hash` implementations need to be improved; only the `env_vars` field of `Evaluator` and the `thread_local` field of `MemoryData` are actually being hashed. Omitting fields from a `Hash` implementation is not incorrect, but could lead to collisions if the ignored fields are changing constantly. Perhaps I should instead derive `Hash` on a few more fields related to MIR validation?
This depends on some of my commits in rustc being merged. I'm not sure how to express this requirement. |
I wouldn't bother with the validation fields, but what about the memory content? Also, this seems extremely costly -- are you essentially storing snapshots to see if one reoccurs? Why should that not eat up all RAM immediately? |
memory content is currently cared about on the rustc side, this is just for extra things that rustc doesn't know |
Seems the rustc side of this got merged, so we should get this landed soon-ish? |
Yea, but we need a new nightly before Travis will accept the PR |
How is it possible that #395 is still green on Travis? |
We didn't get a new nightly |
Thanks :) |
In order to implement infinite loop detection while executing MIR, both the implementer of
Machine
(Evaluator
) and its associated type (MemoryData
), must implementEq
,Clone
andHash
. This PR adds the required trait implementations.It's possible that the
Hash
implementations need to be improved; only theenv_vars
field ofEvaluator
and thethread_local
field ofMemoryData
are actually being hashed. Omitting fields from aHash
implementation is not incorrect, but could lead to collisions if the ignored fields are changing constantly. Perhaps I should instead deriveHash
on a few more fields related to MIR validation?