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

Suboptimal diagnostic if const eval error site is in a different crate #80380

Open
oli-obk opened this issue Dec 26, 2020 · 2 comments
Open

Suboptimal diagnostic if const eval error site is in a different crate #80380

oli-obk opened this issue Dec 26, 2020 · 2 comments
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Dec 26, 2020

We currently produce fun diagnostics like

error: any use of this value will cause an error
  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
   |
LL |     unsafe { copy_nonoverlapping(src, dst, count) }
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |              |
   |              memory access failed: pointer must be in-bounds at offset 8, but is outside bounds of alloc6 which has size 4
   |              inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL
   |              inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
   |              inside `ptr::mut_ptr::<impl *mut u32>::read` at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
   |              inside `_MUT_READ` at $DIR/out_of_bounds_read.rs:15:37
   | 
  ::: $DIR/out_of_bounds_read.rs:15:5
   |
LL |     const _MUT_READ: u32 = unsafe { (PAST_END_PTR as *mut u32).read() };
   |     --------------------------------------------------------------------

Which I have multiple problems with (all of which are my fault 😛)

  1. the main message only makes sense when you realize this is a lint
  2. the diagnostic points to another crate's internals as the main span
  3. the local crate's span is pointing at the entire item, instead of the useful span that we have (last element in the stack trace)

I think we should adjust the diagnostic to report on the site in the currently compiling crate while still mentioning the backtrace in full, so something like

error: memory access failed: pointer must be in-bounds at offset 8, but is outside bounds of alloc6 which has size 
  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
   |
LL |     const _MUT_READ: u32 = unsafe { (PAST_END_PTR as *mut u32).read() };
   |                                                               ^^^^^^^ in function call here
   |     
LL |     unsafe { copy_nonoverlapping(src, dst, count) }
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ failure in external crate here
   |              inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL
   |              inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
   |              inside `ptr::mut_ptr::<impl *mut u32>::read` at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
   |              inside `_MUT_READ` at $DIR/out_of_bounds_read.rs:15:37

cc @rust-lang/wg-diagnostics @rust-lang/wg-const-eval

@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints A-const-eval Area: Constant evaluation (MIR interpretation) labels Dec 26, 2020
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 26, 2020
@RalfJung
Copy link
Member

Also see #71800: the lint should hopefully become a hard error eventually. I am currently working on a PR to make it a future-incompat lint, as a first step.

@RalfJung
Copy link
Member

Some other notes:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants