rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.74k stars 12.5k forks source link

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

Open QuineDot opened 1 year ago

QuineDot commented 1 year ago

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:

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 commented 1 year ago

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

Update: I've posted https://github.com/rust-lang/rust/pull/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 https://github.com/rust-lang/compiler-team/issues/295 describes my overall plan here and my hope is to keep those patterns working long term.

Jules-Bertholet commented 1 year ago

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

apiraino commented 1 year ago

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

RalfJung commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

lcnr commented 5 months ago

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:

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.