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

ICE: lazy_type_alias: cannot relate consts of different types #114456

Closed
matthiaskrgr opened this issue Aug 4, 2023 · 3 comments · Fixed by #125671
Closed

ICE: lazy_type_alias: cannot relate consts of different types #114456

matthiaskrgr opened this issue Aug 4, 2023 · 3 comments · Fixed by #125671
Labels
C-bug Category: This is a bug. F-adt_const_params `#![feature(adt_const_params)]` F-lazy_type_alias `#![feature(lazy_type_alias)]` fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

Code

#![feature(adt_const_params, lazy_type_alias)]

pub type Matrix = [usize; 1];
const EMPTY_MATRIX: Matrix = [0; 1];

pub struct Walk<const REMAINING: Matrix> {}

impl Walk<EMPTY_MATRIX> {
    pub const fn new() -> Self {
        Self {}
    }
}

fn main() {
    let _ = Walk::new();
}

Meta

rustc --version --verbose:

rustc 1.73.0-nightly (474709a9a 2023-08-03)
binary: rustc
commit-hash: 474709a9a2a74a8bcf0055fadb335d0ca0d2d939
commit-date: 2023-08-03
host: x86_64-unknown-linux-gnu
release: 1.73.0-nightly
LLVM version: 16.0.5

Error output

<output>
Backtrace

warning: the feature `adt_const_params` is incomplete and may not be safe to use and/or cause compiler crashes
 --> treereduce.out:1:12
  |
1 | #![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
 --> treereduce.out:1:30
  |
1 | #![feature(adt_const_params, lazy_type_alias)]
  |                              ^^^^^^^^^^^^^^^
  |
  = note: see issue #112792 <https://github.com/rust-lang/rust/issues/112792> for more information

error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: cannot relate consts of different types (a=Const { ty: Matrix, kind: ?0c }, b=Const { ty: [usize; 1], kind: Branch([Leaf(0x0000000000000000)]) })
  |
  = note: delayed at /rustc/474709a9a2a74a8bcf0055fadb335d0ca0d2d939/compiler/rustc_infer/src/infer/combine.rs:178:31
             0: <rustc_errors::HandlerInner>::emit_diagnostic
             1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, alloc::string::String>
             2: <rustc_infer::infer::InferCtxt>::super_combine_consts::<rustc_infer::infer::nll_relate::TypeRelating<rustc_infer::infer::canonical::query_response::QueryTypeRelatingDelegate>>
             3: <rustc_infer::infer::nll_relate::TypeRelating<rustc_infer::infer::canonical::query_response::QueryTypeRelatingDelegate> as rustc_middle::ty::relate::TypeRelation>::consts
             4: <rustc_infer::infer::InferCtxt>::instantiate_nll_query_response_and_region_obligations::<()>
             5: <rustc_middle::ty::ParamEnvAnd<rustc_middle::traits::query::type_op::AscribeUserType> as rustc_trait_selection::traits::query::type_op::TypeOp>::fully_perform
             6: <rustc_borrowck::type_check::TypeChecker>::ascribe_user_type
             7: rustc_borrowck::type_check::type_check
             8: rustc_borrowck::nll::compute_regions
             9: rustc_borrowck::do_mir_borrowck
            10: rustc_borrowck::mir_borrowck
            11: rustc_query_impl::plumbing::__rust_begin_short_backtrace::<rustc_query_impl::query_impl::mir_borrowck::dynamic_query::{closure#2}::{closure#0}, rustc_middle::query::erase::Erased<[u8; 8]>>
            12: <rustc_query_impl::query_impl::mir_borrowck::dynamic_query::{closure#2} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, rustc_span::def_id::LocalDefId)>>::call_once
            13: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, false>
            14: rustc_query_impl::query_impl::mir_borrowck::get_query_non_incr::__rust_end_short_backtrace
            15: rustc_data_structures::sync::par_for_each_in::<&[rustc_span::def_id::LocalDefId], <rustc_middle::hir::map::Map>::par_body_owners<rustc_interface::passes::analysis::{closure#1}::{closure#0}>::{closure#0}>
            16: <rustc_session::session::Session>::time::<(), rustc_interface::passes::analysis::{closure#1}>
            17: rustc_interface::passes::analysis
            18: rustc_query_impl::plumbing::__rust_begin_short_backtrace::<rustc_query_impl::query_impl::analysis::dynamic_query::{closure#2}::{closure#0}, rustc_middle::query::erase::Erased<[u8; 1]>>
            19: <rustc_query_impl::query_impl::analysis::dynamic_query::{closure#2} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, ())>>::call_once
            20: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::SingleCache<rustc_middle::query::erase::Erased<[u8; 1]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, false>
            21: rustc_query_impl::query_impl::analysis::get_query_non_incr::__rust_end_short_backtrace
            22: <rustc_middle::ty::context::GlobalCtxt>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}::{closure#4}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
            23: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_span::ErrorGuaranteed>>
            24: std::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
            25: <<std::thread::Builder>::spawn_unchecked_<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
            26: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/474709a9a2a74a8bcf0055fadb335d0ca0d2d939/library/alloc/src/boxed.rs:2007:9
            27: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/474709a9a2a74a8bcf0055fadb335d0ca0d2d939/library/alloc/src/boxed.rs:2007:9
            28: std::sys::unix::thread::Thread::new::thread_start
                       at /rustc/474709a9a2a74a8bcf0055fadb335d0ca0d2d939/library/std/src/sys/unix/thread.rs:108:17
            29: <unknown>
            30: <unknown>


note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: please attach the file at `/tmp/im/rustc-ice-2023-08-04T10:13:31.528732612Z-1012286.txt` to your bug report

query stack during panic:
end of query stack
error: aborting due to 2 previous errors; 2 warnings emitted

@matthiaskrgr matthiaskrgr added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. F-lazy_type_alias `#![feature(lazy_type_alias)]` labels Aug 4, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 4, 2023
@matthiaskrgr matthiaskrgr added the F-adt_const_params `#![feature(adt_const_params)]` label Aug 4, 2023
@matthiaskrgr
Copy link
Member Author

bisected to introduction in #112853

@fmease
Copy link
Member

fmease commented Aug 4, 2023

Lazy type aliases are not necessary to reproduce this:

#![feature(adt_const_params)]

const EMPTY_MATRIX: <Type as Trait>::Matrix = [0; 1];

pub struct Walk<const REMAINING: <Type as Trait>::Matrix> {}

impl Walk<EMPTY_MATRIX> {
    pub const fn new() -> Self {
        Self {}
    }
}

pub enum Type {}
pub trait Trait { type Matrix; }
impl Trait for Type { type Matrix = [usize; 1]; }

fn main() {
    let _ = Walk::new();
}

@fmease
Copy link
Member

fmease commented Aug 4, 2023

I'd like to clarify, we should definitely keep the label F-lazy_type_alias since this code will regress if / once we make lazy type aliases the default (in a new edition). However, my finding demonstrates that the bug is preexisting. The faulty code path doesn't handle alias types (ty::AliasTy) correctly (ty::Projection in the case of associated types and ty::Weak in the case of lazy type aliases).

@rustbot label -needs-triage

@rustbot rustbot removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 4, 2023
@compiler-errors compiler-errors added the fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. label Aug 6, 2023
@matthiaskrgr matthiaskrgr added the S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. label Apr 15, 2024
fmease added a commit to fmease/rust that referenced this issue May 30, 2024
…iler-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`
@bors bors closed this as completed in 32a3ed2 May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-adt_const_params `#![feature(adt_const_params)]` F-lazy_type_alias `#![feature(lazy_type_alias)]` fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
4 participants