Navigation Menu

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

add Miri to CI #781

Merged
merged 3 commits into from Apr 23, 2019
Merged

add Miri to CI #781

merged 3 commits into from Apr 23, 2019

Conversation

RalfJung
Copy link
Contributor

This adds a CI job running the test suites in Miri -- at least if Miri is available for the current nightly.

Includes #780.

@RalfJung
Copy link
Contributor Author

Oh wow, this worked on the first try. :)

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

Looks good other than the hacks. Lets wait until your other PRs are merged then rebase.

Shame so many tests had to be disabled, but I guess that's the way it is.

src/distributions/gamma.rs Show resolved Hide resolved
src/distributions/poisson.rs Outdated Show resolved Hide resolved
utils/ci/miri.sh Show resolved Hide resolved
@RalfJung
Copy link
Contributor Author

Shame so many tests had to be disabled, but I guess that's the way it is.

Yeah... they don't seem to be around any unsafe code though if I didn't miss anything?

@dhardy
Copy link
Member

dhardy commented Apr 20, 2019

Looks good.

use the UB-free version with proper alignment in Miri

This commit doesn't do what it says any more. But I think the plan is to fix fill_bytes as discussed in #779, then drop this one completely.

@RalfJung
Copy link
Contributor Author

This commit doesn't do what it says any more.

Why doesn't it? It makes it so that on Miri it will use the fallback version (formerly not(any(target_arch = "x86", target_arch = "x86_64"))).

But I think the plan is to fix fill_bytes as discussed in #779, then drop this one completely.

That would be best. Unfortunately, doing that is beyond my knowledge of rand.

@dhardy
Copy link
Member

dhardy commented Apr 23, 2019

So I we rebase this, fingers crossed it'll work perfectly...

@RalfJung
Copy link
Contributor Author

Looking good.

@dhardy dhardy merged commit d998742 into rust-random:master Apr 23, 2019
@dhardy
Copy link
Member

dhardy commented May 2, 2019

We're now seeing the test fail frequently in many PRs:

   Compiling rustc-std-workspace-alloc v1.0.0
error[E0463]: can't find crate for `std`

Is this a case of Miri not being enabled in the nightly build, and if so can we hide the error?

@RalfJung
Copy link
Contributor Author

RalfJung commented May 2, 2019

You are running into rust-lang/miri#713. I will submit a PR.

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.

None yet

2 participants