Rust: Compiler panic when implementing recursive trait for dyn Fn ?

[viftodi]

Hello,

I am trying to implement this trait:

pub trait JsSerializable {
fn size(&self) -> u32;
fn ser(&self, cursor: &mut Cursor>) -> ();
}

impl JsSerializable for &'static Fn() -> (JsSerializable)

I get a compiler panic

thread 'main' panicked at 'assertion failed: !layout.is_unsized()', src\librustc_codegen_ssa\mir\place.rs:45:9
stack backtrace:
   0: std::sys_common::alloc::realloc_fallback
   1: std::panicking::take_hook
   2: std::panicking::take_hook
   3: <rustc::ty::sty::Binder<rustc::ty::ProjectionPredicate<'tcx>> as rustc::ty::ToPredicate<'tcx>>::to_predicate
   4: std::panicking::rust_panic_with_hook
   5: <rustc_codegen_llvm::llvm_::archive_ro::Iter<'a> as core::ops::drop::Drop>::drop
   6: <rustc_codegen_llvm::llvm_::ffi::debuginfo::DIFlags as core::fmt::UpperHex>::fmt
   7: <rustc_codegen_llvm::llvm_::ffi::debuginfo::DIFlags as core::fmt::UpperHex>::fmt
   8: <rustc_codegen_llvm::back::lto::ThinLTOImports as core::fmt::Debug>::fmt
   9: <rustc_codegen_llvm::base::ValueIter<'ll> as core::iter::iterator::Iterator>::next
  10: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::join_codegen_and_link
  11: <rustc_codegen_llvm::base::ValueIter<'ll> as core::iter::iterator::Iterator>::next
  12: <rustc_codegen_llvm::back::lto::ThinLTOImports as core::fmt::Debug>::fmt
  13: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  14: <rustc_driver::pretty::UserIdentifiedItem as core::fmt::Debug>::fmt
  15: rustc_driver::driver::phase_4_codegen
  16: <rustc_driver::pretty::UserIdentifiedItem as core::fmt::Debug>::fmt
  17: <rustc_driver::pretty::UserIdentifiedItem as core::fmt::Debug>::fmt
  18: <rustc_driver::pretty::UserIdentifiedItem as core::fmt::Debug>::fmt
  19: <rustc_driver::pretty::UserIdentifiedItem as core::fmt::Debug>::fmt
  20: rustc_driver::driver::compile_input
  21: rustc_driver::run_compiler
  22: <rustc_driver::profile::trace::Query as core::fmt::Debug>::fmt
  23: rustc_driver::run_compiler
  24: <humantime::duration::Error as std::error::Error>::cause
  25: _rust_maybe_catch_panic
  26: rustc_driver::profile::dump
  27: rustc_driver::main
  28: <unknown>
  29: std::panicking::update_panic_count
  30: _rust_maybe_catch_panic
  31: std::rt::lang_start_internal
  32: <unknown>
  33: <unknown>
  34: BaseThreadInitThunk
  35: RtlUserThreadStart
query stack during panic:
end of query stack

error: internal compiler error: unexpected panic

I try to compile it for target=wasm32-unknown-unknown

note: rustc 1.32.0 (9fda7c223 2019-01-16) running on x86_64-pc-windows-msvc

note: compiler flags: -C debuginfo=2 -C incremental --crate-type lib

[jonas-schievink]

Can you provide a self-contained example that reproduces the ICE?

[viftodi]

Hello,

This is the shortest example I could create:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=69c305c9deff95a201e750cf280f6635

If you just build this you should get the error.
It looks like it works? when not in lib mode, not sure

[hellow554]

Even shorter:

use std::cell::RefCell;

thread_local! {
    static TL: RefCell<Option<Box<dyn Fn() -> ToString>>> = RefCell::new(None);
}

pub fn crash(callback: &'static Fn() -> ToString) {
    TL.with(|cc| {
        *cc.borrow_mut() = Some(Box::new(callback));
    });
}

[hellow554]

Bug was introduced between 1.23 and 1.24 (766bd11c8...4d90ac38c)
Between 2017-11-20 and 2017-11-21 (5041b3bb3...33374fa9d)

[hellow554]

cc @eddyb based on your commits that day ^^

[eddyb]

That example should not type-check, you're assigning a Box<&T> where a Box<T> is expected.

cc @rust-lang/compiler

[estebank]

Using the minimized repro, we have an stable ICE of some sort since 1.27, before then it was rejected due to dyn Trait syntax being unstable:

• 1.27

thread 'main' panicked at 'attempt to divide by zero', librustc_trans/abi.rs:130:26

• 1.28

thread 'main' panicked at 'unsupported integer: Reg { kind: Integer, size: Size { raw: 0 } }', librustc_target/abi/call/mod.rs:147:26

• 1.30

thread 'main' panicked at 'assertion failed: !layout.is_unsized()', librustc_codegen_llvm/mir/place.rs:50:9

• 1.32

thread 'main' panicked at 'assertion failed: !layout.is_unsized()', src/librustc_codegen_ssa/mir/place.rs:45:9

[eddyb]

@estebank You can just remove the dyn keyword, it really shouldn't matter.

Btw, the minimized repro can be minimized further by removing the (rather heavy) thread_local!:

use std::cell::RefCell;

pub fn foo(callback: &'static Fn() -> ToString) {
    let x: RefCell<Option<Box<Fn() -> ToString>>> = RefCell::new(None);
    *x.borrow_mut() = Some(Box::new(callback));
}

And we can also get rid of RefCell:

pub fn foo(callback: &'static Fn() -> ToString) {
    let mut x: Option<Box<Fn() -> ToString>> = None;
    x = Some(Box::new(callback));
}

Fn() -> ToString is dyn Fn<(), Output = dyn ToString>, none of this can work.

So the real question is how does this ICE before emitting a type-checking error?

[eddyb]

Okay, so there are two parts to this:

• the Output associated type of FnOnce doesn't require Sized
• &dyn Fn() implements the Fn() trait, so Box<&dyn Fn()> can be unsized to Box<Fn()>

So this has a reason to type-check, despite being very confusing.

We can make this simpler by using fn pointers:

pub fn foo(callback: fn() -> ToString) {
    let mut x: Option<Box<Fn() -> ToString>> = None;
    x = Some(Box::new(callback));
}

This way, we have fn() -> T: Fn() -> T, so Box<fn() -> T> can be unsized to Box<Fn() -> T>.

But it's impossible for the return type of a function, even with unsized rvalues, to be !Sized (i.e. we have no mechanism for supporting that), and here I think at the very least the vtable of fn() -> T: Fn() -> T has to have a Fn::call shim that would need to return a !Sized return type (T, i.e. ToString in the original repro), so we ICE.

If we don't want to enforce that return types are Sized in fn pointers or trait objects methods, this case will continue ICE-ing.

[eddyb]

The original problem is clearer if we add the dyn keyword:

pub trait JsSerializable {
    fn size(&self) -> u32;
    fn ser(&self, cursor: &mut Cursor<Vec>);
}

impl JsSerializable for &'static dyn Fn() -> dyn JsSerializable {...}

@viftodi You can fix your code by putting a Box<...> around the return type of this Fn(), or figure out some other way to do it.

If you want to make sure you find such problems, try to write function that is has implements the right traits, e.g. fn foo() -> dyn JsSerializable {...}.
That will error because the return type of a function with a body must be Sized.

[estebank]

If we don't want to enforce that return types are Sized in fn pointers or trait objects methods, this case will continue ICE-ing.

What alternative do we have?

[viftodi]

@viftodi You can fix your code by putting a Box<...> around the return type of this Fn(), or figure out some other way to do it.

Thank you @eddyb , I will look into an alternative

[Centril]

Reproduces (https://github.com/rust-lang/rust/issues/58355#issuecomment-463402894) as of 2020-03-10.