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

Unboxed closure ICE due to conflicting use of self param space #18685

Closed
bkoropoff opened this issue Nov 6, 2014 · 0 comments · Fixed by #18688
Closed

Unboxed closure ICE due to conflicting use of self param space #18685

bkoropoff opened this issue Nov 6, 2014 · 0 comments · Fixed by #18688
Labels
A-closures Area: Closures (`|…| { … }`) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@bkoropoff
Copy link
Contributor

Found while debugging #18661

#![feature(unboxed_closures, overloaded_calls)]

trait Tr {
    fn foo(&self);

    fn bar(&self) {
        (|:| { self.foo() })()
    }
}

impl Tr for () {
    fn foo(&self) {}
}

fn main() {
    ().bar();
}

Output:

error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: http://doc.rust-lang.org/complement-bugreport.html
note: run with `RUST_BACKTRACE=1` for a backtrace
task 'rustc' panicked at 'assertion failed: self.self_ty().is_none()', /home/bkoropoff/Projects/rust/src/librustc/middle/subst.rs:161

stack backtrace:
   1:     0x7f248d8634d0 - rt::backtrace::imp::write::hd5cf03b086df3c8bciq
   2:     0x7f248d866550 - failure::on_fail::h1883f65e4859f99aODq
   3:     0x7f248e014550 - unwind::begin_unwind_inner::h4724b673057aa7b2ZSd
   4:     0x7f248e3ee820 - unwind::begin_unwind::h14014107635648236996
   5:     0x7f248e73efe0 - middle::subst::Substs::with_self_ty::h2bc29c972ac1c03fKHV
   6:     0x7f248e787cb0 - middle::trans::meth::trans_method_callee::h577047eddd14a758DZm
   7:     0x7f248e787c30 - middle::trans::controlflow::trans_for::closure.122893
   8:     0x7f248ed45840 - middle::trans::expr::trans_overloaded_call::closure.123736
   9:     0x7f248e7862f0 - middle::trans::callee::trans_call_inner::h63140c053eb498d4Tr4
  10:     0x7f248e7b8c30 - middle::trans::expr::trans_rvalue_dps_unadjusted::hcfd8c05e4d3c050aOA6
  11:     0x7f248e77e510 - middle::trans::expr::trans_into::h513a5ff934428ca7Nb5
  12:     0x7f248e77e930 - middle::trans::controlflow::trans_block::h1f8437f97d982780ys1
  13:     0x7f248e81fa20 - middle::trans::base::trans_closure::hee28be544b5f82b7eAh
  14:     0x7f248e771b70 - middle::trans::base::trans_fn::h4c36ddee4bfba9759Lh
  15:     0x7f248e772280 - middle::trans::monomorphize::monomorphic_fn::h9b8dc1f03343f037YS0
  16:     0x7f248e7a4330 - middle::trans::callee::trans_fn_ref_with_substs::h0704b46911f5f6dd953
  17:     0x7f248e787cb0 - middle::trans::meth::trans_method_callee::h577047eddd14a758DZm
  18:     0x7f248e7aafd0 - middle::trans::callee::trans_method_call::closure.123334
  19:     0x7f248e7862f0 - middle::trans::callee::trans_call_inner::h63140c053eb498d4Tr4
  20:     0x7f248e7aaad0 - middle::trans::callee::trans_method_call::h983e88dde8c4f64fln4
  21:     0x7f248e7b8c30 - middle::trans::expr::trans_rvalue_dps_unadjusted::hcfd8c05e4d3c050aOA6
  22:     0x7f248e77e510 - middle::trans::expr::trans_into::h513a5ff934428ca7Nb5
  23:     0x7f248e77d970 - middle::trans::controlflow::trans_stmt_semi::hc8263f70f5ea8146Fr1
  24:     0x7f248e77cf20 - middle::trans::controlflow::trans_stmt::h4ab602fde4741168sn1
  25:     0x7f248e77e930 - middle::trans::controlflow::trans_block::h1f8437f97d982780ys1
  26:     0x7f248e81fa20 - middle::trans::base::trans_closure::hee28be544b5f82b7eAh
  27:     0x7f248e771b70 - middle::trans::base::trans_fn::h4c36ddee4bfba9759Lh
  28:     0x7f248e76f290 - middle::trans::base::trans_item::h671a84973f340984n5h
  29:     0x7f248e82a420 - middle::trans::base::trans_crate::hbe1167d59e4442a8s3i
  30:     0x7f248ec6ab70 - driver::driver::phase_4_translate_to_llvm::hd4dc6d8be6267a8b5sB
  31:     0x7f248ec61360 - driver::driver::compile_input::h8b6b9b7f64eba9a5ZZA
  32:     0x7f248ece5c20 - driver::run_compiler::hf2410821f04b1325cQE
  33:     0x7f248ece5b10 - driver::run::closure.145823
  34:     0x7f248e4266e0 - task::TaskBuilder<S>::try_future::closure.104097
  35:     0x7f248e4264d0 - task::TaskBuilder<S>::spawn_internal::closure.104068
  36:     0x7f248f55e370 - task::NativeSpawner.Spawner::spawn::closure.8434
  37:     0x7f248e06a4d0 - rust_try_inner
  38:     0x7f248e06a4c0 - rust_try
  39:     0x7f248e011ea0 - unwind::try::h2a8363f131a2efbejHd
  40:     0x7f248e011d30 - task::Task::run::ha950d1ffc07a77f09Mc
  41:     0x7f248f55e0b0 - task::NativeSpawner.Spawner::spawn::closure.8372
  42:     0x7f248e013540 - thread::thread_start::hc29987e17333695ao8c
  43:     0x7f248d32fe20 - start_thread
  44:     0x7f248dcdeb59 - clone
  45:                0x0 - <unknown>

This is due to the self param space being used both for the unboxed closure itself within the implicit FnOnce impl, and for the self type of the enclosing trait.

I don't think the first use is ever necessary since the self of the unboxed closure cannot actually be mentioned within it. The unboxed closure type may need to be plumbed through some other way, though.

@huonw huonw added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-closures Area: Closures (`|…| { … }`) labels Nov 6, 2014
bkoropoff added a commit to bkoropoff/rust that referenced this issue Nov 7, 2014
- When selecting an implicit trait impl for an unboxed closure, plumb
  through and use the substitutions from impl selection instead of
  using those from the current param environment in trans, which may
  be incorrect.
- When generating a function declaration for an unboxed closure, plumb
  through the substitutions from the param environment of the closure
  as above.  Also normalize the type to avoid generating duplicate
  declarations due to regions being inconsistently replaced with
  ReStatic elsewhere.
- Do not place the closure type in the self param space when
  translating the unboxed closure callee, etc.  It is not actually
  used, and doing so conflicts with the self substitution from
  default trait methods.

Closes rust-lang#18661
Closes rust-lang#18685
bkoropoff added a commit to bkoropoff/rust that referenced this issue Nov 7, 2014
bors added a commit that referenced this issue Nov 7, 2014
…nikomatsakis

This resolves some issues that remained after adding support for monomorphizing unboxed closures in trans.

There were a few places where a set of substitutions for an unboxed closure type were dropped on the floor and later recalculated from scratch based on the def ID, but this failed spectacularly when the closure originated from a different param environment.  The substitutions are now plumbed through end-to-end.  Closes #18661

There was also a conflict in the meaning of the self param space within the body of the unboxed closure.  Trans attempted to insert the unboxed closure type as the self type, but this could conflict with the self type from the param environment when an unboxed closure was used within a default method on a trait.  Since the body of an unboxed closure cannot refer to its own self type or value, there's no need for it to actually use the self space.  The downstream consumers of the substitutions in trans do not seem to need it either since they look up the type of the closure some other way, so I just stopped setting it.  Closes #18685.

r? @pcwalton @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants