Skip to content
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

fix miri #25

Merged
merged 5 commits into from
Jan 10, 2025
Merged

fix miri #25

merged 5 commits into from
Jan 10, 2025

Conversation

hsqStephenZhang
Copy link
Contributor

fix all miri errors in this PR

@@ -725,7 +725,6 @@ mod test {
}

#[test]
#[cfg_attr(miri, ignore)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it has been fixed, so remove the ignore tag

@@ -1363,7 +1363,7 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> RawLRU<K, V, E, S> {
{
match self.map.get_mut(KeyWrapper::from_ref(k)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may refer to https://github.com/jeromefroe/lru-rs/blob/2d18d2d333cee29057a23f065533d3f2b8dc0ae0/src/lib.rs#L1031.
this kind of operation shall all be accessed via pointers than references

.unwrap();

self.attach(bks.as_mut());
let node = self.map.remove(&old_key).unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get the actual node by map.remove, so the value shall be valid

})
.unwrap();

self.attach(bks.as_mut());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

@@ -1618,6 +1618,7 @@ mod test {
use rand::{thread_rng, Rng};
use std::format;

#[cfg_attr(miri, ignore)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the loop is costly, just ignore

@al8n
Copy link
Owner

al8n commented Jan 10, 2025

Thank you very much!

@al8n al8n merged commit 3a66328 into al8n:main Jan 10, 2025
27 of 32 checks passed
@al8n al8n mentioned this pull request Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants