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

Extra type parameters make compiler panic calling `Option::unwrap()` on `None` #36954

Closed
raphaelcohn opened this issue Oct 4, 2016 · 15 comments

Comments

@raphaelcohn
Copy link

commented Oct 4, 2016

There's been a change in the Rust nightly compiler sometime between 2016-09-15 and 2016-09-28 (both inclusive) that causes it to ICE on the code in this repository. Compiling on Mac OS X El Capitan, using TARGET_CC=x86_64-linux-musl-cc exec cargo build --target=x86_64-unknown-linux-musl (this assumes brew install FiloSottile/musl-cross/musl-cross using Homebrew).

There are no code changes. A backtrace shows the following:-

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: run with `RUST_BACKTRACE=1` for a backtrace

thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', ../src/libcore/option.rs:323
stack backtrace:
   1:        0x1127ec288 - std::sys::backtrace::tracing::imp::write::hb03a0f85d9cccd1b
   2:        0x1127f99ff - std::panicking::default_hook::{{closure}}::h945d25329ef3fdd6
   3:        0x1127f6efd - std::panicking::default_hook::hc98de28314f7e4d1
   4:        0x1127f7596 - std::panicking::rust_panic_with_hook::hf47d6a84575c96f5
   5:        0x1127f7434 - std::panicking::begin_panic::heb2e59d774c89ce6
   6:        0x1127f7352 - std::panicking::begin_panic_fmt::h2640a71eaf059834
   7:        0x1127f72b7 - rust_begin_unwind
   8:        0x112837640 - core::panicking::panic_fmt::h8e7400f6438ecb75
   9:        0x112837544 - core::panicking::panic::h61585b9964ba1fa2
  10:        0x10f3e6e05 - rustc_const_eval::eval::eval_const_expr_partial::h27ecb783f31aa113
  11:        0x10f3e53d5 - rustc_const_eval::eval::eval_const_expr_partial::h27ecb783f31aa113
  12:        0x10f3e3375 - rustc_const_eval::eval::eval_const_expr_partial::h27ecb783f31aa113
  13:        0x10f3d6b5f - rustc::hir::Pat::walk_::h112ba77c5a1884b6
  14:        0x10f3ee2d3 - rustc_const_eval::check_match::check_expr::h0ec31ffaf8981f57
  15:        0x10f3ed38a - rustc_const_eval::check_match::check_expr::h0ec31ffaf8981f57
  16:        0x10f3ed38a - rustc_const_eval::check_match::check_expr::h0ec31ffaf8981f57
  17:        0x10f3fb3fa - rustc_const_eval::check_match::check_fn::h86267ad0b244e886
  18:        0x10f3ec59a - rustc_const_eval::check_match::check_crate::hfbd5f166424f830a
  19:        0x10e6ca528 - rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}::h24ab4d83fd97a88e
  20:        0x10e695296 - rustc_driver::driver::phase_3_run_analysis_passes::h007ecd1fd02a4683
  21:        0x10e68829a - rustc_driver::driver::compile_input::h8a5ca0f49da8c80a
  22:        0x10e6b0188 - rustc_driver::run_compiler::ha0d125e702c7be63
  23:        0x10e5f0bb0 - std::panicking::try::do_call::h25f69b2f74f3cecc
  24:        0x1127f9fba - __rust_maybe_catch_panic
  25:        0x10e610594 - <F as alloc::boxed::FnBox<A>>::call_box::h18252cde177b90c5
  26:        0x1127f61b4 - std::sys::thread::Thread::new::thread_start::h66211a1b34bcffec
  27:     0x7fff8f34e99c - _pthread_body
  28:     0x7fff8f34e919 - _pthread_start

When the code compiles with the older Rust compiler, the output is:-

warning: lint drop_with_repr_extern has been removed: drop flags have been removed
  |
  = note: requested on the command line with `-D drop_with_repr_extern`

warning: unused import, #[warn(unused_imports)] on by default
  --> src/wrapper/ringQueues/mod.rs:16:5
   |
16 | use ::wrapper::ringQueues::consumers::SingleConsumer;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import, #[warn(unused_imports)] on by default
  --> src/wrapper/ringQueues/mod.rs:17:5
   |
17 | use ::wrapper::ringQueues::consumers::MultiConsumer;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import, #[warn(unused_imports)] on by default
  --> src/wrapper/ringQueues/mod.rs:23:5
   |
23 | use ::wrapper::ringQueues::producers::SingleProducer;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused import, #[warn(unused_imports)] on by default
  --> src/wrapper/ringQueues/mod.rs:24:5
   |
24 | use ::wrapper::ringQueues::producers::MultiProducer;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'll try to triage further.

@TimNN TimNN added the I-ICE label Oct 4, 2016

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

OK, nightly compiler 2016-09-21 does not ICE, narrowing the range to >2016-09-21.
Trying more nightly variants...

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

OK, nightly compiler 2016-09-22 does not ICE either.

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

OK, nightly compiler 2016-09-27 does ICE. rustup tells me there are no nightly toolchains to download for Sep 2016 23, 24, 25 or 26.

@TimNN

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

Minimized:

lib.rs:

#![feature(const_fn)]
#![crate_type = "lib"]

const fn foo(i: i32) -> i32 {
    i
}

pub const FOO: i32 = foo(1);

main.rs:

extern crate lib;

fn main() {
    let _ = lib::FOO;
}

The unwrap seems to be this one.

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

Wow - that's impressive finding the cause that quickly.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Oct 4, 2016

@eddyb

This comment has been minimized.

Copy link
Member

commented Oct 4, 2016

We don't seem to have any tests for cross-crate const fn used in early const eval (enum discriminants or array lengths), so my change from NodeId to DefId in Def::Local broke this without being noticed.

The fix would involve using DefId instead of NodeId as keys for the fn_args map, with the creation of said map getting the key by tcx.expect_def(arg.pat.id).def_id(), going through the identity Def::Local which points the pattern back to its original DefId in the source crate.

Such ugly hacks will go away once @solson merges miri and we hook it up for early const eval.

@eddyb eddyb added the E-mentor label Oct 4, 2016

@HeroesGrave

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2016

I think I might be hitting this issue too but my project is too big to track down the cause properly.

@datariot

This comment has been minimized.

Copy link

commented Oct 11, 2016

I get the same stack backtrace on macOS 10.12.1 Beta (16B2338c) with the nightly. The same code compiles just fine in an ubuntu docker container.

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Oct 20, 2016

@eddyb Any chance of a short term fix or reversion of change? Please? I know const fn isn't particularly stable, but trying to write code without it is painful. Actually, writing code with it is also painful, given its restrictions, but I digress...

@eddyb

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

@raphaelcohn I'm sorry if it wasn't clear, I described a quick fix in my comment and am willing to mentor it on IRC.

@raphaelcohn

This comment has been minimized.

Copy link
Author

commented Nov 3, 2016

@eddyb Sorry for the late reply, I've been child rearing over half term. I appreciate the offer to mentor it, but I've not got the current time to commit. If someone else wants to take up your offer (@datariot, @HeroesGrave perhaps?) I'd be happy to test a fix.

Since this is a blocker for my team, we may fork the upstream crates we depend on and strip out their use of const fn. We'll also have a re-think about how we might go about supporting our current project once it hits live, given our need for a number of unstable features. Looks like we might need to have core compiler expertise on the team - something we weren't planning on.

bors added a commit that referenced this issue Nov 3, 2016
Auto merge of #37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes #36954.

r? @eddyb

cc @raphaelcohn
jonathandturner added a commit to jonathandturner/rust that referenced this issue Nov 4, 2016
Rollup merge of rust-lang#37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes rust-lang#36954.

r? @eddyb

cc @raphaelcohn
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Nov 4, 2016

I'm tagging with E-help-wanted in an effort to bring more attention to it.

I'm also nominating for @rust-lang/compiler to discuss whether to bump this to P-high. It is a regression, but only affecting const fn, which is unstable. Still, I consider const fn to be something we plan to support long-term and we obviously ought to fix it. I hate to think of it blocking @raphaelcohn.

@eddyb

This comment has been minimized.

Copy link
Member

commented Nov 4, 2016

@nikomatsakis Thanks, but #37557 is in the queue already.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Nov 4, 2016

Argh, I see it now. For some reason I always overlook those "citations". Sorry for the noise.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 4, 2016
Rollup merge of rust-lang#37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes rust-lang#36954.

r? @eddyb

cc @raphaelcohn
bors added a commit that referenced this issue Nov 5, 2016
Auto merge of #37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes #36954.

r? @eddyb

cc @raphaelcohn
jonathandturner added a commit to jonathandturner/rust that referenced this issue Nov 5, 2016
Rollup merge of rust-lang#37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes rust-lang#36954.

r? @eddyb

cc @raphaelcohn
bors added a commit that referenced this issue Nov 5, 2016
Auto merge of #37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes #36954.

r? @eddyb

cc @raphaelcohn
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 5, 2016
Rollup merge of rust-lang#37557 - TimNN:fix-36954, r=eddyb
Use DefId's in const eval for cross-crate const fn's

Fixes rust-lang#36954.

r? @eddyb

cc @raphaelcohn

@bors bors closed this in #37557 Nov 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.