Skip to content

Fix stacked borrow violation and add miri CI - #25

Merged
dtolnay merged 3 commits into
masterfrom
borrow
Apr 23, 2020
Merged

Fix stacked borrow violation and add miri CI#25
dtolnay merged 3 commits into
masterfrom
borrow

Conversation

@dtolnay

@dtolnay dtolnay commented Apr 23, 2020

Copy link
Copy Markdown
Owner

Fixes #24.

dtolnay added 3 commits April 22, 2020 17:19
Miri flagged that we borrow a reference to just the first element and
then write through it to elements other than the first. The new
implementation borrows the whole buffer before writing.
@dtolnay
dtolnay merged commit f7cd2ea into master Apr 23, 2020
@dtolnay
dtolnay deleted the borrow branch April 23, 2020 00:38
Comment thread .travis.yml
name: Miri
script:
- rustup component add miri || travis_terminate 0
- cargo miri test

@RalfJung RalfJung Apr 23, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great to see Miri being added here. :)

cargo miri test will interactively ask for confirmation to install things, and will probably not work. Miri is just not available for the latest nightly so right now we do not enter this code path.
The recommended Miri CI integration is given in our README.

Comment thread tests/d2s_test.rs

#[test]
fn test_random() {
let n = if cfg!(miri) { 100 } else { 1000000 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, I like this style... so far I always used #[cfg(miri)] for this but that means we need 4 lines to define one constant.

Comment thread tests/d2s_test.rs
}
}

#[cfg(not(miri))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FWIW, the usual alternative I use these days is #[cfg_attr(miri, ignore)]. Then at least it is clear in the output that we are skipping some tests.

It doesn't really make a difference, though.

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.

Miri reports stacked borrowing error in unsafe cast from element to array

2 participants