make miri happy #36
make miri happy #36
Conversation
this makes pointers exist in the borrow stack of the entire buffer instead of a single element
this creates a single unique reference to the segments, thus avoiding invalidation of pointers
| let d = sv.drain((Bound::Included(1), Bound::Included(i))); | ||
| let drained = d.map(|i| i.1).collect::<Vec<_>>(); | ||
| assert_eq!(dc.get(), i, "i={}", i); | ||
| assert_eq!(sv.len(), 8 - i, "i={}", i); | ||
| assert_eq!(drained, (2..=(i as i32 + 1)).collect::<Vec<_>>(), "i={}", i); | ||
| } | ||
| } | ||
|
|
||
| for dc in to_cleanup.into_iter() { |
There was a problem hiding this comment.
This is fine, but I don't think it is strictly necessary - the memory allocated for these leaked Box values is negligible, and this code only runs within the tests, and this cleanup code won't run if the test fails anyway.
There was a problem hiding this comment.
i agree - it's just that miri will complain about leaked values if these are not deallocated
| @@ -613,8 +631,10 @@ fn test_segmented_iter() { | |||
|
|
|||
| #[test] | |||
| fn test_sort() { | |||
| const COUNT: usize = if cfg!(miri) { 32 } else { 1000 }; | |||
There was a problem hiding this comment.
No issues here, just wondering if this is a "takes too long" issue or a "miri actually breaks here" issue (I haven't run it in a long time)
There was a problem hiding this comment.
takes too long, yeah. the 32 is arbitrary, however
|
Do you have any interest in modifying the CI code to run the tests under |
|
sure! i'll do it together with the safety comments in a few hours. |
fixes #29 and others
it would be good to add
// SAFETY: ...comments - i'll probably add some in a few hours.