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

Valiation: Identify write locks using an abstract lvalue #336

Merged
merged 5 commits into from Sep 13, 2017

Conversation

Projects
None yet
3 participants
@RalfJung
Copy link
Member

RalfJung commented Sep 13, 2017

Fixes #296

identify write locks by lvalues, not regions
This makes a new compile-fail test pass.
}

fn test(r: &mut RefCell<i32>) {
let x = &*r; // releasing write lock, first suspension recorded

This comment has been minimized.

@oli-obk

oli-obk Sep 13, 2017

Collaborator

If you want you can try using ui tests together with MIRI_LOG=rust_miri::interpret::validate=trace to be able to ensure that the trace does exactly what you want.

But I guess it would fail with an interpretation error if anything went wrong...

This comment has been minimized.

@RalfJung

RalfJung Sep 13, 2017

Author Member

The test fails prior to the PR, so I think compile-fail is enough here.

@@ -1466,7 +1466,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> {

/// ensures this Value is not a ByRef
pub(super) fn follow_by_ref_value(
&mut self,
&self,

This comment has been minimized.

@oli-obk

oli-obk Sep 13, 2017

Collaborator

❤️

This comment has been minimized.

@RalfJung

RalfJung Sep 13, 2017

Author Member

Is there any way to get lints for "You used &mut but could have used &"?^^

This comment has been minimized.

@oli-obk

oli-obk Sep 13, 2017

Collaborator

No. But I've been trying to write a lint for that since forever: rust-lang/rust-clippy#353

This comment has been minimized.

@eddyb

eddyb Sep 13, 2017

Member

It should be easier on MIR because you can look for uses that aren't immutable reborrows.

@RalfJung RalfJung force-pushed the RalfJung:mir-validate branch from ba66359 to 59a329d Sep 13, 2017

@RalfJung

This comment has been minimized.

Copy link
Member Author

RalfJung commented Sep 13, 2017

I am somewhat confused by these zst2.rs and zst3.rs compile-fail tests. First of all, why were they compile-fail in the first place? They are checking that some assertion fails; might as well check that the assertion holds...

...except, it turns out, miri disagrees with rustc here. The following passes on miri, and fails on rustc:

#[derive(Debug)]
struct A;

fn main() {
    // can't use assert_eq, b/c that will try to print the pointer addresses with full MIR enabled
    assert!(&A as *const A as *const () != &() as *const _);
    assert!(&A as *const A != &A as *const A);
}

Is this a bug? Why would we deliberately test for disagreeing with rustc?

@oli-obk

This comment has been minimized.

Copy link
Collaborator

oli-obk commented Sep 13, 2017

Is this a bug? Why would we deliberately test for disagreeing with rustc?

We don't deduplicate constants in all cases: #131

@RalfJung

This comment has been minimized.

Copy link
Member Author

RalfJung commented Sep 13, 2017

All right, I will then disable the test if it is buggy.

@RalfJung RalfJung force-pushed the RalfJung:mir-validate branch from 5699376 to 1c96472 Sep 13, 2017

@RalfJung RalfJung force-pushed the RalfJung:mir-validate branch from 1c96472 to 5d2ed4d Sep 13, 2017

@RalfJung RalfJung merged commit 237590a into rust-lang:master Sep 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.