Changing the structure of `mir::interpret::InterpError` by saleemjaffer · Pull Request #62969 · rust-lang/rust

saleemjaffer · 2019-07-25T11:30:26Z

Implements this

rust-highfive · 2019-07-25T11:30:30Z

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

saleemjaffer · 2019-07-25T11:30:43Z

r? @RalfJung @oli-obk

src/librustc/mir/interpret/error.rs

oli-obk

Just some nits then this lgtm.

src/librustc_mir/interpret/cast.rs

src/librustc_mir/interpret/eval_context.rs

src/librustc_mir/interpret/memory.rs

RalfJung · 2019-07-31T16:56:26Z

@saleemjaffer if you think this is ready for reviews, please remove the "WIP". ;)

RalfJung

Thanks a lot!

r=me with these nits fixed.

src/librustc/mir/interpret/mod.rs

src/librustc_mir/interpret/eval_context.rs

src/librustc_mir/transform/const_prop.rs

src/librustc/mir/interpret/mod.rs

src/librustc_mir/transform/const_prop.rs

RalfJung · 2019-08-01T06:26:39Z

r=me

;-)
This convention means that as far as I am concerned, this PR is good to go with the nits fixed -- so anyone with bors powers could then do at-bors r=RalfJung.

src/librustc_mir/transform/const_prop.rs

saleemjaffer · 2019-08-01T06:45:21Z

@bors r= RalfJung

bors · 2019-08-01T06:45:22Z

@saleemjaffer: 🔑 Insufficient privileges: Not in reviewers

saleemjaffer · 2019-08-01T06:46:14Z

@RalfJung I do not have bors privileges XD. So I cannot do this.

RalfJung · 2019-08-01T07:14:42Z

That's okay. :) I use r=me either way, so I don't have to figure out in advance if someone has bors powers.^^

@bors r+

bors · 2019-08-01T07:14:43Z

📌 Commit 00d32e8 has been approved by RalfJung

… r=RalfJung Changing the structure of `mir::interpret::InterpError` Implements [this](rust-lang/const-eval#4 (comment))

@ghost

Rollup of 7 pull requests Successful merges: - #62663 (More questionmarks in doctests) - #62969 (Changing the structure of `mir::interpret::InterpError`) - #63153 (Remove redundant method with const variable resolution) - #63189 (Doc improvements) - #63198 (Allow trailing comma in macro 2.0 declarations.) - #63202 (Fix ICE in #63135) - #63203 (Make is_mutable use PlaceRef instead of it's fields) Failed merges: r? @ghost

RalfJung · 2019-08-02T15:40:36Z

@saleemjaffer yay, it landed. :)

However, as expected, this also broke Miri. Do you want to try and fix that?
You need to clone Miri from https://github.com/rust-lang/miri/ and install the latest (later-than-nightly) rustc with https://github.com/kennytm/rustup-toolchain-install-master. In the Miri src directory:

rustup-toolchain-install-master -c rust-src
rustup override set <hash> (the hash is shown by the previous step)
edit the rust-version file to also just contain that hash
run ./miri test; this will fail because the err! macro is gone -- fix those errors, similar to what you did in rustc
make a PR :)

RalfJung · 2019-08-03T08:03:07Z

Never mind, I'll do it because this blocks other work.

Thanks for the PR!

@oli-obk

A bit of Miri error cleanup Some cleanup after rust-lang#62969. r? @oli-obk Cc @saleemjaffer

rust-highfive assigned matthewjasper Jul 25, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2019

rust-highfive assigned RalfJung and unassigned matthewjasper Jul 25, 2019

saleemjaffer mentioned this pull request Jul 25, 2019

Making enums more specific in src/librustc/mir/interpret/error.rs rust-lang/const-eval#4

Closed

4 tasks

This comment has been minimized.

Sign in to view

RalfJung reviewed Jul 25, 2019

View reviewed changes