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

evaluation error hides "impl item missing" error #98629

Closed
lcnr opened this issue Jun 28, 2022 · 6 comments · Fixed by #99449
Closed

evaluation error hides "impl item missing" error #98629

lcnr opened this issue Jun 28, 2022 · 6 comments · Fixed by #99449
Assignees
Labels
A-const-generics Area: const generics (parameters and arguments) A-diagnostics Area: Messages for errors, warnings, and lints P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Jun 28, 2022

trait Trait {
    const N: usize;
}

impl Trait for i32 {}

fn f()
where
    [(); <i32 as Trait>::N]:,
{}

fn main() {}

results in

error[[E0080]](https://doc.rust-lang.org/nightly/error-index.html#E0080): evaluation of constant value failed
 --> src/main.rs:9:10
  |
9 |     [(); <i32 as Trait>::N]:,
  |          ^^^^^^^^^^^^^^^^^ referenced constant has errors

without showing that N is missing in the trait impl.

Taken from the comment by @carado in #74713 (comment)

@lcnr lcnr added the A-diagnostics Area: Messages for errors, warnings, and lints label Jun 28, 2022
@BoxyUwU BoxyUwU added the A-const-generics Area: const generics (parameters and arguments) label Jun 28, 2022
@Rageking8
Copy link
Contributor

@rustbot label T-compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 28, 2022
@Rageking8
Copy link
Contributor

Possible regression from stable to nightly. (Tested in Rust Playground)

Both Stable 1.61.0 and 1.62.0-beta.7 (2022-06-26 747075d) result in this error :

error[E0046]: not all trait items implemented, missing: `N`
 --> src/main.rs:5:1
  |
2 |     const N: usize;
  |     --------------- `N` from trait
...
5 | impl Trait for i32 {}
  | ^^^^^^^^^^^^^^^^^^ missing `N` in implementation

For more information about this error, try `rustc --explain E0046`.

However on Nightly version: 1.64.0-nightly (2022-06-27 2f3ddd9)

This is the error :

error[E0080]: evaluation of constant value failed
 --> src/main.rs:9:10
  |
9 |     [(); <i32 as Trait>::N]:,
  |          ^^^^^^^^^^^^^^^^^ referenced constant has errors

For more information about this error, try `rustc --explain E0080`.

@Rageking8
Copy link
Contributor

@rustbot label regression-from-stable-to-nightly

@rustbot rustbot added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 28, 2022
@apiraino
Copy link
Contributor

if my bisection is correct, commit 1f34da9 seems to trigger this. In #96591 cc @b-naber

searched nightlies: from nightly-2022-04-01 to nightly-2022-06-28
regressed nightly: nightly-2022-06-15
searched commits: from ca122c7 to 1f34da9
regressed commit: 1f34da9

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2022-04-01 --script=./test.sh 

$ cat test.sh 
#!/bin/bash
cargo check 2>&1 | grep "from trait"

@TaKO8Ki TaKO8Ki self-assigned this Jun 30, 2022
@apiraino
Copy link
Contributor

apiraino commented Jun 30, 2022

WG-prioritization assigning priority (Zulip discussion).

The missing diagnostic message in this case can possibly make fixing the code quite confusing. Thanks @TaKO8Ki for offering some help here :)

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 30, 2022
@compiler-errors
Copy link
Member

compiler-errors commented Jul 15, 2022

Hi @TaKO8Ki, are you working on this issue? I've been keeping tabs on this one for a bit and I decided to look into it since it's been a few weeks.

I think I may have found a bug in the way that we resolve const items and I would like to put up a PR, unless you've already found a solution. 😸

Also, marking this as a regression from stable to beta, since beta cutoff has happened since this issue was filed, I think.

@compiler-errors compiler-errors added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 15, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 20, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? `@oli-obk` or `@lcnr` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 21, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? ``@oli-obk`` or ``@lcnr`` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 21, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? ```@oli-obk``` or ```@lcnr``` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 21, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? ````@oli-obk```` or ````@lcnr```` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 22, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? `@oli-obk` or `@lcnr` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 23, 2022
…-item, r=lcnr

Do not resolve associated const when there is no provided value

Fixes rust-lang#98629, since now we just delay a bug when we're not able to evaluate a const item due to the value not actually being provided by anything. This means compilation proceeds forward to where the "missing item in impl" error is emitted.

----

The root issue here is that when we're looking for the defining `LeafDef` in `resolve_associated_item`, we end up getting the trait's AssocItem instead of the impl's AssocItem (which does not exist). This resolution "succeeds" even if the trait's item has no default value, and then since this item has no value to evaluate, it turns into a const eval error.

This root issue becomes problematic (as in rust-lang#98629) when this const eval error happens in wfcheck (for example, due to normalizing the param-env of something that references this const). Since this happens sooner than the check that an impl actually provides all of the items that a trait requires (which happens during later typecheck), we end up aborting compilation early with only this un-informative message.

I'm not exactly sure _why_ this bug arises due to rust-lang#96591 -- perhaps valtrees are evaluated more eagerly than in the old system?

r? `@oli-obk` or `@lcnr` since y'all are familiar with const eval and reviewed rust-lang#96591, though feel free to reassign.

This is a regression from stable to beta, so I would be open to considering this for beta backport. It seems correct to me, especially given the improvements in the other UI tests this PR touches, but may have some side-effects that I'm unaware of...?
@bors bors closed this as completed in 3648dd5 Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) A-diagnostics Area: Messages for errors, warnings, and lints P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants