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

Fix ret > 1 bound if shadowed by const #93593

Merged
merged 1 commit into from
Feb 4, 2022
Merged

Conversation

JulianKnodt
Copy link
Contributor

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc @compiler-errors
Should Fix #93553

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2022
.filter(|r| self.trait_defines_associated_type_named(r.def_id(), assoc_name));
let mut const_candidates = all_candidates()
.filter(|r| self.trait_defines_associated_const_named(r.def_id(), assoc_name));

Copy link
Contributor Author

@JulianKnodt JulianKnodt Feb 2, 2022

Choose a reason for hiding this comment

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

This is the main fix, the rest should make it more carefully stick to logic prior associated-const equality change which introduced the ice.

@compiler-errors
Copy link
Member

Can you verify that the compiler doesn't ICE in situations like:

trait Baz {
  const Bar: usize;
  const Qux: Self::Bar;
}

@JulianKnodt
Copy link
Contributor Author

I don't think it should ice in that case since there's no shadowing and no name overlaps, but will add a test in

@JulianKnodt
Copy link
Contributor Author

oh it does ice, I'll look into it a bit further

@compiler-errors
Copy link
Member

I think the stack trace points to an .expect so you can find where it ICEs. I think it's in a method that resolves a path to a ty, maybe you want to check if it resolves to a const instead and emit an error. Maybe something like this:

trait Baz {
  const Bar: usize;
  const Qux: Self::Bar;
//           ^ Expected type, found const
}

@JulianKnodt
Copy link
Contributor Author

JulianKnodt commented Feb 3, 2022

yuh, i'm not sure if the best way to fix it is to turn the expect into an actual error, or handle it earlier, for now I can just turn it into an error and if the reviewer wants to do the other I'll update

compiler/rustc_typeck/src/astconv/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/astconv/mod.rs Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Feb 3, 2022

r? @oli-obk

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.
@oli-obk
Copy link
Contributor

oli-obk commented Feb 3, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Feb 3, 2022

📌 Commit 2dfd77d 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 Feb 3, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
Fix ret > 1 bound if shadowed by const

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc `@compiler-errors`
Should Fix rust-lang#93553
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
Fix ret > 1 bound if shadowed by const

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc ``@compiler-errors``
Should Fix rust-lang#93553
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
Fix ret > 1 bound if shadowed by const

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc ```@compiler-errors```
Should Fix rust-lang#93553
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 4, 2022
Fix ret > 1 bound if shadowed by const

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc ````@compiler-errors````
Should Fix rust-lang#93553
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 4, 2022
Fix ret > 1 bound if shadowed by const

Prior to a change, it would only look at types in bounds. When it started looking for consts,
shadowing type variables with a const would cause an ICE, so now defer looking at consts only if
there are no types present.

cc `````@compiler-errors`````
Should Fix rust-lang#93553
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2022
…askrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#92735 (Add crate filter parameter in URL)
 - rust-lang#93402 (Windows: Disable LLVM crash dialog boxes.)
 - rust-lang#93508 (Add rustdoc info to jsondocck output)
 - rust-lang#93551 (Add package.json in gitignore)
 - rust-lang#93555 (Link `try_exists` docs to `Path::exists`)
 - rust-lang#93585 (Missing tests for rust-lang#92630)
 - rust-lang#93593 (Fix ret > 1 bound if shadowed by const)
 - rust-lang#93630 (clippy::perf fixes)
 - rust-lang#93631 (rustc_mir_dataflow: use iter::once instead of Some().into_iter)
 - rust-lang#93632 (rustdoc: clippy::complexity fixes)
 - rust-lang#93638 (rustdoc: remove unused Hash impl)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 92a7f5f into rust-lang:master Feb 4, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Associated const with supertrait associated type (named the same) causes ICE
7 participants