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

broken MIR in DefId(...) (NoSolution) #110534

Open
QuineDot opened this issue Apr 19, 2023 · 9 comments
Open

broken MIR in DefId(...) (NoSolution) #110534

QuineDot opened this issue Apr 19, 2023 · 9 comments
Labels
C-bug Category: This is a bug. fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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

@QuineDot
Copy link

Code

Playground.

use core::cell::Ref;

struct System;

trait IntoSystem { 
    fn into_system(self) -> System;
}

impl IntoSystem for fn(Ref<'_, u32>) {
    fn into_system(self) -> System { System }
}

impl<A> IntoSystem for fn(A)
where
    // n.b. No `Ref<'_, u32>` can satisfy this bound
    A: 'static + for<'x> MaybeBorrowed<'x, Output = A>,
{
    fn into_system(self) -> System { System }
}

//---------------------------------------------------

trait MaybeBorrowed<'a> {
    type Output: 'a;
}

// If you comment this out you'll see the compiler chose to look at the
// fn(A) implementation of IntoSystem above
impl<'a, 'b> MaybeBorrowed<'a> for Ref<'b, u32> {
    type Output = Ref<'a, u32>;
}

// ---------------------------------------------

fn main() {
    fn sys_ref(_age: Ref<u32>) {}
    let _sys_c = (sys_ref as fn(_)).into_system();
    // properly fails
    // let _sys_c = (sys_ref as fn(Ref<'static, u32>)).into_system();
    // properly succeeds
    // let _sys_c = (sys_ref as fn(Ref<'_, u32>)).into_system();
}

Meta

Playground:

  • Build using the Stable version: 1.68.2
  • Build using the Beta version: 1.69.0-beta.8 (2023-04-13 f18236d)
  • Build using the Nightly version: 1.71.0-nightly (2023-04-16 d0f204e)

It wasn't an ICE in 1.62.

Error output

warning: conflicting implementations of trait `IntoSystem` for type `for<'a> fn(Ref<'a, u32>)`
  --> src/main.rs:13:1
   |
9  | impl IntoSystem for fn(Ref<'_, u32>) {
   | ------------------------------------ first implementation here
...
13 | impl<A> IntoSystem for fn(A)
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'a> fn(Ref<'a, u32>)`
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, [see issue #56105 <https://github.com/rust-lang/rust/issues/56105>](https://github.com/rust-lang/rust/issues/56105)
   = note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details
   = note: `#[warn(coherence_leak_check)]` on by default

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

error: internal compiler error: broken MIR in DefId(0:21 ~ playground[c84b]::main) (NoSolution): could not prove Binder(TraitPredicate(<fn(std::cell::Ref<'_, u32>) as IntoSystem>, polarity:Positive), [])
  |
  = note: delayed at    0: <rustc_errors::HandlerInner>::emit_diagnostic
             1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, &str>
             2: <rustc_borrowck::type_check::TypeChecker>::normalize_and_prove_instantiated_predicates
             3: <rustc_borrowck::type_check::TypeVerifier as rustc_middle::mir::visit::Visitor>::visit_constant
             4: <rustc_borrowck::type_check::TypeVerifier as rustc_middle::mir::visit::Visitor>::visit_body
             5: rustc_borrowck::nll::compute_regions
             6: rustc_borrowck::do_mir_borrowck
             7: rustc_borrowck::mir_borrowck
             8: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::mir_borrowck, rustc_query_impl::plumbing::QueryCtxt>
             9: 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#2}::{closure#0}>::{closure#0}>
            10: <rustc_session::session::Session>::time::<(), rustc_interface::passes::analysis::{closure#2}>
            11: rustc_interface::passes::analysis
            12: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::queries::analysis, rustc_query_impl::plumbing::QueryCtxt>
            13: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::analysis
            14: <std::thread::local::LocalKey<core::cell::Cell<*const ()>>>::with::<rustc_middle::ty::context::tls::enter_context<<rustc_middle::ty::context::GlobalCtxt>::enter<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}::{closure#4}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
            15: <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>>
            16: rustc_span::set_source_map::<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}::{closure#0}>
            17: 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>>
            18: <<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}
            19: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/d0f204e4d750b62f9d6c2593405e828757126832/library/alloc/src/boxed.rs:1973:9
            20: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                       at /rustc/d0f204e4d750b62f9d6c2593405e828757126832/library/alloc/src/boxed.rs:1973:9
            21: std::sys::unix::thread::Thread::new::thread_start
                       at /rustc/d0f204e4d750b62f9d6c2593405e828757126832/library/std/src/sys/unix/thread.rs:108:17
            22: start_thread
            23: clone
          

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: rustc 1.71.0-nightly (d0f204e4d 2023-04-16) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type bin -C embed-bitcode=no -C codegen-units=1 -C debuginfo=2

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
warning: `playground` (bin "playground") generated 1 warning
error: could not compile `playground` (bin "playground"); 1 warning emitted
@QuineDot QuineDot added C-bug Category: This is a bug. 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. labels Apr 19, 2023
@QuineDot
Copy link
Author

Note that while there is a coherence_leak_check warning here, Niko's last comment on the issue says:

Update: I've posted #72493. It affects this tracking issue in that it makes some patterns into hard-errors. However, the patterns we've seen reported thus far (which are actually the same pattern) still work with a warning. The MCP rust-lang/compiler-team#295 describes my overall plan here and my hope is to keep those patterns working long term.

@Jules-Bertholet
Copy link
Contributor

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

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

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 14, 2023
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Jun 25, 2023
@RalfJung
Copy link
Member

RalfJung commented Mar 18, 2024

Wow that's a wild one... not sure whom to Cc here. @tmiasko @compiler-errors for some general MIR knowledge, and @lcnr since traits seem involved somehow.

The error is a bit different on latest stable

error: internal compiler error: error performing ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing }, value: ProvePredicate { predicate: Binder { value: TraitPredicate(<fn(std::cell::Ref<'_, u32>) as IntoSystem>, polarity:Positive), bound_vars: [] } } }
  --> src/main.rs:37:18
   |
37 |     let _sys_c = (sys_ref as fn(_)).into_system();
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: delayed at /rustc/eb45c844407968ea54df0d9870ebce9e3235b706/compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs:165:29

@compiler-errors
Copy link
Member

Huh. I'm surprised that this doesn't cause a trait error in HIR typeck. It does if we use a type alias to avoid introducing a higher ranked fn ptr and actually ascribe the fn(Ref<'?0, u32>)...:

type Fun<A> = fn(A);

fn main() {
    fn sys_ref(_age: Ref<u32>) {}
    let _sys_c = (sys_ref as Fun<Ref<'_, u32>>).into_system();
    //~^ ERROR  no method named `into_system` found for fn pointer `fn(Ref<'_, u32>)` in the current scope

@compiler-errors
Copy link
Member

Woo, this is due to a lot of fun things.

Firstly, we defer the cast of sys_ref as fn(_) until after the rest of typeck, which means that we only know that the receiver of .into_system() is fn(?0). That registers a fn(?0): IntoSystem goal, which does pass the leak check in method probing because we haven't constrained ?0.

After we apply the cast, we do end up with fn(Ref<'?1, u32>): IntoSystem goal, which itself doesn't do any leak check because this is fulfillment and not evaluation.

@compiler-errors compiler-errors added the fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. label Mar 18, 2024
@compiler-errors
Copy link
Member

compiler-errors commented Mar 19, 2024

Also, I guess this also has nothing to do with method probing technically:

// Borrowing the trait definitions from above above

fn foo<T>(_: Option<T>) where T: IntoSystem {}

fn main() {
    let x = None::<fn(_)>;
    foo(x);
    let y: Option<fn(Ref<'static, u32>)> = x;
}

... which still ICEs during mir borrowck.

@lcnr
Copy link
Contributor

lcnr commented Mar 19, 2024

we may want to leak check at the end of hir typeck. Feels like the easiest fix 😁

@lcnr
Copy link
Contributor

lcnr commented Mar 19, 2024

minimized:

trait Trait {}
impl<T: for<'a> LeakErr<'a>> Trait for T {}
impl<U: for<'a> LeakErr<'a>> Trait for Option<U> {}

trait LeakErr<'a> {}
impl<T> LeakErr<'static> for T {}

fn impls_trait<T: Trait>(x: T) -> T {
    x
}

fn main() {
    let y = impls_trait(None);
    let _: Option<u32> = y;
}

What's happening here is the following:

  • evaluation uses the leak check in evaluation_probe
  • selection uses evaluation only if there are multiple applicable candidates
  • fulfillment never uses the leak check

In HIR typeck, we select Option<?x>: Trait which has two applicable candidates, causing us to use evaluate_candidate. for<'a> Option<?x>: LeakErr<'a> of the first impl causes a leak check error. for<'a> ?x: LeakErr<'a> of the second impl remains ambiguous. This causes us to select the second impl. We then only prove for<'a> u32: LeakErr<'a> in fulfillment which does not use the leak check. THis causes HIR typeck to pass.

In MIR typeck, we select Option<u32>: Trait which has two applicable candidates, causing us to use evaluate_candidate. However, now both candidates fail the leak check, causing us to have no applicable impl, causing a selection error.

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. fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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
Development

No branches or pull requests

9 participants