forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 7
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of rust-lang#125671 - BoxyUwU:remove_const_ty_eq, r=compil…
…er-errors Do not equate `Const`'s ty in `super_combine_const` Fixes rust-lang#114456 In rust-lang#125451 we started relating the `Const`'s tys outside of a probe so it was no longer simply an assertion to catch bugs. This was done so that when we _do_ provide a wrongly typed const argument to an item if we wind up relating it with some other instantiation we'll have a `TypeError` we can bubble up and taint the resulting mir allowing const eval to skip evaluation. In this PR I instead change `ConstArgHasType` to correctly handle checking the types of const inference variables. Previously if we had something like `impl<const N: u32> Trait for [(); N]`, when using the impl we would instantiate it with infer vars and then check that `?x: u32` is of type `u32` and succeed. Then later we would infer `?x` to some `Const` of type `usize`. We now stall on `?x` in `ConstArgHasType` until it has a concrete value that we can determine the type of. This allows us to fail using the erroneous implementation of `Trait` which allows us to taint the mir. Long term we intend to remove the `ty` field on `Const` so we would have no way of accessing the `ty` of a const inference variable anyway and would have to do this. I did not fully update `ConstArgHasType` to avoid using the `ty` field as it's not entirely possible right now- we would need to lookup `ConstArgHasType` candidates in the env. --- As for _why_ I think we should do this, relating the types of const's is not necessary for soundness of the type system. Originally this check started off as a plain `==` in `super_relate_consts` and gradually has been growing in complexity as we support more complicated types. It was never actually required to ensure that const arguments are correctly typed for their parameters however. The way we currently check that a const argument has the correct type is a little convoluted and confusing (and will hopefully be less weird as time goes on). Every const argument has an anon const with its return type set to type of the const parameter it is an argument to. When type checking the anon const regular type checking rules require that the expression is the same type as the return type. This effectively ensure that no matter what every const argument _always_ has the correct type. An extra bit of complexity is that during `hir_ty_lowering` we do not represent everything as a `ConstKind::Unevaluated` corresponding to the anon const. For generic parameters i.e. `[(); N]` we simply represent them as `ConstKind::Param` as we do not want `ConstKind::Unevaluated` with generic substs on stable under min const generics. The anon const still gets type checked resulting in errors about type mismatches. Eventually we intend to not create anon consts for all const arguments (for example for `ConstKind::Param`) and instead check that the argument type is correct via `ConstArgHasType` obligations (these effectively also act as a check that the anon consts have the correctly set return type). What this all means is that the the only time we should ever have mismatched types when relating two `Const`s is if we have messed up our logic for ensuring that const arguments are of the correct type. Having this not be an assert is: - Confusing as it may incorrectly lead people to believe this is an important check that is actually required - Opens the possibility for bugs or behaviour reliant on this (unnecessary) check existing --- This PR makes two tests go from pass->ICE (`generic_const_exprs/ice-125520-layout-mismatch-mulwithoverflow.rs` and `tests/crashes/121858.rs`). This is caused by the fact that we evaluate anon consts even if their where clauses do not hold and is a pre-existing issue and only affects `generic_const_exprs`. I am comfortable exposing the brokenness of `generic_const_exprs` more with this PR This PR makes a test go from ICE->pass (`const-generics/issues/issue-105821.rs`). I have no idea why this PR affects that but I believe that ICE is an unrelated issue to do with the fact that under `generic_const_exprs`/`adt_const_params` we do not handle lifetimes in const parameter types correctly. This PR is likely just masking this bug. Note: this PR doesn't re-introduce the assertion that the two consts' tys are equal. I'm not really sure how I feel about this but tbh it has caused more ICEs than its found lately so 🤷♀️ r? `@oli-obk` `@compiler-errors`
- Loading branch information
Showing
30 changed files
with
206 additions
and
327 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
7 changes: 2 additions & 5 deletions
7
tests/ui/consts/eval_type_mismatch.rs → tests/crashes/121858.rs
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,14 @@ | ||
//@ known-bug: #121858 | ||
#![feature(generic_const_exprs)] | ||
#![allow(incomplete_features)] | ||
|
||
struct Outer<const A: i64, const B: usize>(); | ||
impl<const A: usize, const B: usize> Outer<A, B> | ||
//~^ ERROR: `A` is not of type `i64` | ||
//~| ERROR: mismatched types | ||
where | ||
[(); A + (B * 2)]:, | ||
{ | ||
fn o() {} | ||
fn o() -> Union {} | ||
} | ||
|
||
fn main() { | ||
Outer::<1, 1>::o(); | ||
//~^ ERROR: no function or associated item named `o` found | ||
} |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,23 @@ | ||
//@ known-bug: #123141 | ||
trait ConstChunksExactTrait<T> { | ||
fn const_chunks_exact<const N: usize>(&self) -> ConstChunksExact<'_, T, { N }>; | ||
|
||
trait Trait { | ||
fn next(self) -> Self::Item; | ||
type Item; | ||
} | ||
|
||
impl<T> ConstChunksExactTrait<T> for [T] {} | ||
struct Foo<T: ?Sized>(T); | ||
|
||
struct ConstChunksExact<'a, T: 'a, const N: usize> {} | ||
impl<T: ?Sized, U> Trait for Foo<U> { | ||
type Item = Foo<T>; | ||
fn next(self) -> Self::Item { | ||
loop {} | ||
} | ||
} | ||
|
||
impl<'a, T, const N: usize> Iterator for ConstChunksExact<'a, T, { rem }> { | ||
type Item = &'a [T; N]; | ||
fn opaque() -> impl Trait { | ||
Foo::<_>(10_u32) | ||
} | ||
|
||
fn main() { | ||
let slice = &[1i32, 2, 3, 4, 5, 6, 7, 7, 9, 1i32]; | ||
|
||
let mut iter = [[1, 2, 3], [4, 5, 6], [7, 8, 9]].iter(); | ||
|
||
for a in slice.const_chunks_exact::<3>() { | ||
assert_eq!(a, iter.next().unwrap()); | ||
} | ||
opaque().next(); | ||
} |
This file contains 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
This file contains 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
4 changes: 3 additions & 1 deletion
4
tests/crashes/114456.rs → ...dt_const_params/alias_const_param_ty-1.rs
This file contains 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
19 changes: 19 additions & 0 deletions
19
tests/ui/const-generics/adt_const_params/alias_const_param_ty-1.stderr
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
warning: the feature `adt_const_params` is incomplete and may not be safe to use and/or cause compiler crashes | ||
--> $DIR/alias_const_param_ty-1.rs:2:12 | ||
| | ||
LL | #![feature(adt_const_params, lazy_type_alias)] | ||
| ^^^^^^^^^^^^^^^^ | ||
| | ||
= note: see issue #95174 <https://github.com/rust-lang/rust/issues/95174> for more information | ||
= note: `#[warn(incomplete_features)]` on by default | ||
|
||
warning: the feature `lazy_type_alias` is incomplete and may not be safe to use and/or cause compiler crashes | ||
--> $DIR/alias_const_param_ty-1.rs:2:30 | ||
| | ||
LL | #![feature(adt_const_params, lazy_type_alias)] | ||
| ^^^^^^^^^^^^^^^ | ||
| | ||
= note: see issue #112792 <https://github.com/rust-lang/rust/issues/112792> for more information | ||
|
||
warning: 2 warnings emitted | ||
|
This file contains 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
11 changes: 11 additions & 0 deletions
11
tests/ui/const-generics/adt_const_params/alias_const_param_ty-2.stderr
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
warning: the feature `adt_const_params` is incomplete and may not be safe to use and/or cause compiler crashes | ||
--> $DIR/alias_const_param_ty-2.rs:2:12 | ||
| | ||
LL | #![feature(adt_const_params)] | ||
| ^^^^^^^^^^^^^^^^ | ||
| | ||
= note: see issue #95174 <https://github.com/rust-lang/rust/issues/95174> for more information | ||
= note: `#[warn(incomplete_features)]` on by default | ||
|
||
warning: 1 warning emitted | ||
|
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.