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

A bit more polish on const eval errors #85767

Merged
merged 2 commits into from
May 29, 2021
Merged

Conversation

lqd
Copy link
Member

@lqd lqd commented May 27, 2021

This PR adds a bit more polish to the const eval errors:

  • a slight improvement to the PME messages from Post-monomorphization errors traces MVP #85633: I mentioned there that the erroneous item's paths were dependent on the environment, and could be displayed fully qualified or not. This can obscure the items when they come from a dependency. This PR uses the pretty-printing code ensuring the items' paths are not trimmed.
  • whenever there are generics involved in an item where const evaluation errors out, the error message now displays the instance and its const arguments, so that we can see which instantiated item and compile-time values lead to the error.

So we get this slight improvement for our beloved stdarch example, on nightly:

error[E0080]: evaluation of constant value failed
 --> ./stdarch/crates/core_arch/src/macros.rs:8:9
  |
8 |         assert!(IMM >= MIN && IMM <= MAX, "IMM value not in expected range");
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'IMM value not in expected range', /rustc/9111b8ae9793f18179a1336417618fc07a9cac85/library/core/src/../../stdarch/crates/core_arch/src/macros.rs:8:9
  |

to this PR's:

error[E0080]: evaluation of `core::core_arch::macros::ValidateConstImm::<51_i32, 0_i32, 15_i32>::VALID` failed
 --> ./stdarch/crates/core_arch/src/macros.rs:8:9
  |
8 |         assert!(IMM >= MIN && IMM <= MAX, "IMM value not in expected range");
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'IMM value not in expected range', ./stdarch/crates/core_arch/src/macros.rs:8:9
  |

with this PR.

Of course this is an idea from Oli, so maybe r? @oli-obk if they have the time.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 27, 2021
@oli-obk
Copy link
Contributor

oli-obk commented May 27, 2021

cc @rust-lang/wg-const-eval

@@ -1,4 +1,4 @@
error[E0080]: evaluation of constant value failed
error[E0080]: evaluation of `test::<0_usize>::{constant#0}` failed
Copy link
Contributor

Choose a reason for hiding this comment

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

we already report such paths (with {constant#0}) in other situations. It's not ideal, but maybe better than not mentioning the path?

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference, there are indeed a few cases in the ui tests where this happens today, for example

::: $DIR/issue-80742.rs:23:10
|
LL | [u8; size_of::<T>() + 1]: ,
| -------------- inside `Inline::<dyn Debug>::{constant#0}` at $DIR/issue-80742.rs:23:10

(After/if this PR is merged, I'll work on changing these messages: to keep the originating span, but not the full instance path, so that they are not duplicated)

@oli-obk
Copy link
Contributor

oli-obk commented May 29, 2021

@bors r+

@bors
Copy link
Contributor

bors commented May 29, 2021

📌 Commit c31ca9a has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2021
@bors
Copy link
Contributor

bors commented May 29, 2021

⌛ Testing commit c31ca9a with merge 9f75dbf...

@bors
Copy link
Contributor

bors commented May 29, 2021

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 9f75dbf to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 29, 2021
@bors bors merged commit 9f75dbf into rust-lang:master May 29, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 29, 2021
@lqd lqd deleted the stackless_span_stacks branch May 29, 2021 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants