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

Compiler unnecessary panicked due to a mistake in where clause #40294

Closed
Bendrien opened this issue Mar 6, 2017 · 7 comments · Fixed by #40404
Closed

Compiler unnecessary panicked due to a mistake in where clause #40294

Bendrien opened this issue Mar 6, 2017 · 7 comments · Fixed by #40404
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Bendrien
Copy link

Bendrien commented Mar 6, 2017

The compiler unexpectedly panicked where a nice error should be produced.

I tried this code:

pub fn zip<K, V, W, A, B>(a: A, b: B)
    where K: Ord,
          A: Iterator<Item=(K, V)>,
          A: Iterator<Item=(K, W)>,
{

}

I simply forgot to change the third entry in the where clause to B.

Meta

rustc --version --verbose:
rustc 1.17.0-nightly (24a70eb 2017-02-09)
binary: rustc
commit-hash: 24a70eb
commit-date: 2017-02-09
host: x86_64-pc-windows-gnu
release: 1.17.0-nightly
LLVM version: 3.9

Backtrace:
thread 'rustc' panicked at 'Box', C:\bot\slave\nightly-dist-rustc-win-gnu-64\build\src\librustc_errors\lib.rs:417
stack backtrace:
0: 0x659e854a - strncmp
1: 0x659f2fa7 - strncmp
2: 0x659f2b21 - strncmp
3: 0x659f352e - strncmp
4: 0x66242f9a - strncmp
5: 0x6625b044 - strncmp
6: 0x6f387f5e - strncmp
7: 0x6f387cb5 - strncmp
8: 0x6f3874fc - strncmp
9: 0x6f29c278 - strncmp
10: 0x6f39578b - strncmp
11: 0x6f38af55 - strncmp
12: 0x6f389971 - strncmp
13: 0x70cab10f - strncmp
14: 0x70bea5d6 - strncmp
15: 0x70c6d966 - strncmp
16: 0x70c8097a - strncmp
17: 0x70c97768 - strncmp
18: 0x70cfd984 - strncmp
19: 0x69c60b83 - strncmp
20: 0x69bbc0ab - strncmp
21: 0x69c4ace4 - strncmp
22: 0x69c343a9 - strncmp
23: 0x69c7f2b9 - strncmp
24: 0x69b8af05 - strncmp
25: 0x659f6938 - strncmp
26: 0x69ba83b4 - strncmp
27: 0x659f0d52 - strncmp
28: 0x7ffff6398363 - strncmp

@Bendrien Bendrien changed the title Unnecessary panick due to where clause Compiler unnecessary panicked due to mistake in where clause Mar 6, 2017
@Bendrien Bendrien changed the title Compiler unnecessary panicked due to mistake in where clause Compiler unnecessary panicked due to a mistake in where clause Mar 6, 2017
@TimNN TimNN added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 6, 2017
@TimNN
Copy link
Contributor

TimNN commented Mar 6, 2017

Backtrace from playground:

rustc 1.17.0-nightly (b1e31766d 2017-03-03)
error: internal compiler error: /checkout/src/librustc/hir/map/mod.rs:722: expected expr, found fn zip (id=4)

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 'Box<Any>', /checkout/src/librustc_errors/lib.rs:417
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: rustc_errors::Handler::bug
   1: rustc::session::opt_span_bug_fmt::{{closure}}
   2: rustc::session::opt_span_bug_fmt
   3: rustc::session::bug_fmt
   4: rustc::hir::map::Map::expect_expr
   5: rustc::traits::error_reporting::<impl rustc::infer::InferCtxt<'a, 'gcx, 'tcx>>::need_type_info
   6: rustc::traits::error_reporting::<impl rustc::infer::InferCtxt<'a, 'gcx, 'tcx>>::report_fulfillment_error
   7: rustc::traits::error_reporting::<impl rustc::infer::InferCtxt<'a, 'gcx, 'tcx>>::report_fulfillment_errors
   8: rustc_typeck::check::FnCtxt::select_all_obligations_or_error
   9: <std::thread::local::LocalKey<T>>::with
  10: rustc_typeck::check::wfcheck::CheckTypeWellFormedVisitor::check_item_well_formed
  11: rustc_typeck::check::check_wf_new
  12: rustc_typeck::check_crate
  13: rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}
  14: rustc_driver::driver::phase_3_run_analysis_passes
  15: rustc_driver::driver::compile_input
  16: rustc_driver::run_compiler
  17: std::panicking::try::do_call
  18: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  19: <F as alloc::boxed::FnBox<A>>::call_box
  20: std::sys::imp::thread::Thread::new::thread_start
             at /checkout/src/liballoc/boxed.rs:648
             at /checkout/src/libstd/sys_common/thread.rs:21
             at /checkout/src/libstd/sys/unix/thread.rs:84
  21: start_thread
  22: clone

@eddyb
Copy link
Member

eddyb commented Mar 9, 2017

See #39361 (comment) for how to fix in the compiler (if anyone wants to pick this up).

@nikomatsakis
Copy link
Contributor

triage: P-high

@rust-highfive rust-highfive added P-high High priority and removed I-nominated labels Mar 9, 2017
@nikomatsakis nikomatsakis added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Mar 9, 2017
@nikomatsakis
Copy link
Contributor

Good chance to make a simple, high-impact fix. =) The problem is that this line of code assumes that body_id always refers to a HIR expression (part of our AST), but that's not correct.

The right thing to do would be to match on the results of self.tcx.hir.find(cause.body_id) and check for a couple different variants:

  • a NodeExpr(e) (defined in rustc::hir::map) can just return e
  • for a NodeItem(i), we can match on i.node and see if it is a ItemFn, and then extract the body-id. This is kind of painful, a better way might be to extend the associated_body helper that runs on entries.

OK, not totally trivial, but I don't have time for more in-depth notes just yet. =)

@eddyb
Copy link
Member

eddyb commented Mar 9, 2017

@nikomatsakis I don't think we need to handle anything other than NodeExpr.
That is, this is only adding some extra notes on an error, we can just skip them otherwise.

@nikomatsakis
Copy link
Contributor

Never mind, the easy fix is to do something like this:

let opt_expr = match self.tcx.hir.find(cause.body_id) {
    rustc::hir::map::NodeExpr(e) => Some(e),
    _ => None,
}

then down below where we do visit_expr(e), just check if it is Some. Or move this code down there to the place where we invoke visit_expr

@nikomatsakis
Copy link
Contributor

@eddyb great minds think alike...

alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 10, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 11, 2017
fix rust-lang#40294 obligation cause.body_id is not always a NodeExpr

Hello!

This fixes rust-lang#40294 and moves tests related to rust-lang#38812 to a much more sensible directory.

Thanks to @nikomatsakis and @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants