more specific errors in src/librustc/mir/interpret/error.rs#60951
Merged
bors merged 4 commits intorust-lang:masterfrom Jul 23, 2019
Merged
more specific errors in src/librustc/mir/interpret/error.rs#60951bors merged 4 commits intorust-lang:masterfrom
bors merged 4 commits intorust-lang:masterfrom
Conversation
Contributor
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Contributor
Author
|
r? @oli-obk |
Contributor
Author
|
@oli-obk I have just copy pasted the enum specified in the RFC, so that we can discuss how to do stuff over here. |
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Contributor
|
The current error enum is defined in https://github.com/rust-lang/rust/blob/b4aba66b0577bcd2228afd01421b136c72965b28/src/librustc/mir/interpret/error.rs#L334 You should not copy the one from the RFC issue, but modify the current one, as there may have been significant changes. The RFC issue only serves as a guideline on what to do |
|
ping from triage @saleemjaffer any updates? |
Dylan-DPC-zz
added
S-waiting-on-author
Contributor
Author
|
@Dylan-DPC I was occupied with something over the last two weeks. I can pick this up again in a while. |
|
ping from triage @saleemjaffer |
Dylan-DPC-zz
added
S-inactive
Contributor
Author
|
@oli-obk @Dylan-DPC I can work on this now :) |
saleemjaffer
force-pushed
the
mir_better_error_enum
branch
from
b4aba66 to
efb7457
Compare
Contributor
Author
|
@oli-obk Don't know why does it say that I have closed the PR. Could you reopen it? I have tried doing some refactor, but it does not show up on this PR. Github asks me to open a new PR. |
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Contributor
Author
|
@oli-obk I have tried to start off by changing the enum fields as per the RFC. How do I proceed now? |
oli-obk
reviewed
Jul 18, 2019
Contributor
oli-obk
left a comment
oli-obk
left a comment
There was a problem hiding this comment.
The next steps would be to make all uses of the moved variants compile again.
src/librustc/mir/interpret/error.rs
Outdated
| required: Align, | ||
| has: Align, | ||
| }, | ||
| MemoryLockViolation { |
Contributor
There was a problem hiding this comment.
these suddenly showed up in this PR
src/librustc/mir/interpret/error.rs
Outdated
| DanglingPointerDeref, | ||
| DoubleFree, | ||
| InvalidMemoryAccess, | ||
| FunctionPointerTyMismatch(FnSig<'tcx>, FnSig<'tcx>), |
Contributor
There was a problem hiding this comment.
this wasn't here before this PR, maybe a rebase gone wrong?
src/librustc/mir/interpret/error.rs
Outdated
|
|
||
| #[derive(Clone, RustcEncodable, RustcDecodable)] | ||
| pub enum EvalErrorPanic<'tcx, O> { | ||
| Panic, |
Contributor
There was a problem hiding this comment.
this needs the fields of the old Panic variant from above.
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
saleemjaffer
force-pushed
the
mir_better_error_enum
branch
from
61fc12e to
ed6c5cd
Compare
Dylan-DPC-zz
removed
the
S-inactive
Contributor
Author
|
@oli-obk How does it look? |
saleemjaffer
force-pushed
the
mir_better_error_enum
branch
from
ed6c5cd to
90426ed
Compare
oli-obk
requested changes
Jul 23, 2019
Contributor
oli-obk
left a comment
oli-obk
left a comment
There was a problem hiding this comment.
Just two small things, then it's ready to go
| | OverflowNeg | ||
| | DivisionByZero | ||
| | RemainderByZero | ||
| | Panic(EvalErrorPanic::Panic { .. }) |
Contributor
There was a problem hiding this comment.
just use Panic(_) instead of listing all of these.
This code here is one of the reasons we want a Panic variant
src/librustc/mir/interpret/error.rs
Outdated
| pub type AssertMessage<'tcx> = InterpError<'tcx, mir::Operand<'tcx>>; | ||
|
|
||
| #[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] | ||
| pub enum EvalErrorPanic<O> { |
Contributor
There was a problem hiding this comment.
Name this enum PanicMessage. This way it makes more sense when we'll start reusing it in TerminatorKind::Assert's msg field
Contributor
Author
|
@oli-obk Made the changes. |
oli-obk
changed the title
Contributor
|
Thanks! @bors r+ If you are interested in continuing this avenue, you can (on top of this PR, but in a new PR and branch) change the type of the |
Collaborator
|
📌 Commit 3730ed9 has been approved by |
bors
added
the
S-waiting-on-bors
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Jul 23, 2019
… r=oli-obk more specific errors in src/librustc/mir/interpret/error.rs Implements [this](rust-lang/const-eval#4)
bors
added a commit
that referenced
this pull request
Jul 23, 2019
Rollup of 14 pull requests Successful merges: - #60951 (more specific errors in src/librustc/mir/interpret/error.rs) - #62523 (Delay bug to resolve HRTB ICE) - #62656 (explain how to search in slice without owned data) - #62791 (Handle more cases of typos misinterpreted as type ascription) - #62804 (rustc_typeck: improve diagnostics for _ const/static declarations) - #62808 (Revert "Disable stack probing for gnux32.") - #62817 (Tweak span for variant not found error) - #62842 (Add tests for issue-58887) - #62851 (move unescape module to rustc_lexer) - #62859 (Place::as_place_ref is now Place::as_ref) - #62869 (add rustc_private as a proper language feature gate) - #62880 (normalize use of backticks in compiler messages for librustc_allocator) - #62885 (Change "OSX" to "macOS") - #62889 (Update stage0.txt) Failed merges: r? @ghost
tesuji
added a commit
to tesuji/miri
that referenced
this pull request
Jul 24, 2019
tesuji
added a commit
to tesuji/miri
that referenced
this pull request
Jul 24, 2019
4 tasks
bors
added a commit
to rust-lang/miri
that referenced
this pull request
Jul 24, 2019
build: Fix build after rust-lang/rust#60951
bors
added a commit
to rust-lang/miri
that referenced
this pull request
Jul 24, 2019
build: Fix build after rust-lang/rust#60951
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements this