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

rustfmt no longer builds after rust-lang/rust#77306 #78079

Closed
rust-highfive opened this issue Oct 18, 2020 · 11 comments · Fixed by #78156
Closed

rustfmt no longer builds after rust-lang/rust#77306 #78079

rust-highfive opened this issue Oct 18, 2020 · 11 comments · Fixed by #78156
Assignees
Labels
A-rustfmt Area: Rustfmt C-bug Category: This is a bug.

Comments

@rust-highfive
Copy link
Collaborator

Hello, this is your friendly neighborhood mergebot.
After merging PR #77306, I observed that the tool rustfmt no longer builds.
A follow-up PR to the repository https://github.com/rust-lang/rustfmt is needed to fix the fallout.

cc @lcnr, do you think you would have time to do the follow-up work?
If so, that would be great!

@rust-highfive rust-highfive added A-rustfmt Area: Rustfmt C-bug Category: This is a bug. labels Oct 18, 2020
@matthewjasper
Copy link
Contributor

 thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Binder(<smallvec::SmallVec<[u8; _]> as std::ops::Index<std::ops::RangeFull>>)`,
 right: `Binder(<smallvec::SmallVec<[u8; 36]> as std::ops::Index<std::ops::RangeFull>>)`', compiler/rustc_trait_selection/src/traits/codegen/mod.rs:30:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

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

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.49.0-nightly (4d247ad7d 2020-10-18) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z macro-backtrace -Z binary-dep-depinfo -C opt-level=3 -C embed-bitcode=no -C debuginfo=0 -C debug-assertions=on -C overflow-checks=off -C link-args=-Wl,-rpath,$ORIGIN/../lib --crate-type lib

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [codegen_fulfill_obligation] checking if `std::ops::Index` fulfills its obligations
#1 [resolve_instance] resolving instance `<smallvec::SmallVec<[u8; _]> as std::ops::Index<std::ops::RangeFull>>::index`
end of query stack
[RUSTC-TIMING] rustc_ap_rustc_data_structures test:false 1.603
error: could not compile **`rustc-ap-rustc_data_structures`**

@bishtpawan
Copy link
Contributor

Hi @matthewjasper, can you please the command to replicate this bug? Thanks

@matthewjasper
Copy link
Contributor

.\x.py build .\src\tools\rustfmt\ with debug-assertions = true set in config.toml

@lcnr
Copy link
Contributor

lcnr commented Oct 19, 2020

won't be able to get to this myself in the next few days. To fix this we probably have to compile it with RUST_BACKTRACE=1, find the place where we call Instance::resolve and put a tcx.normalize_erasing_regions before that

@bishtpawan
Copy link
Contributor

Hi @lcnr, the error occurred where we just erasing regions by simply doing this (tcx.erase_regions(&trait_ref)) and provided assertion with (tcx.normalize_erasing_regions(param_env, trait_ref)). Is this scenario correct? Just want to confirm as you already mentioned to put a tcx.normalize_erasing_regions. Thanks

@lcnr
Copy link
Contributor

lcnr commented Oct 19, 2020

yes, so we do not want to use tcx.normalize_erasing_regions in codegen_fulfill_obligations as that can be a bit costly but instead normalize eagerly before then.

To detect cases where we are still using unnormalized values #77306 added a debug assert there.
If we look at the backtrace we can find out where the trait_ref is coming from and normalize it there

@bishtpawan
Copy link
Contributor

okay, thanks for the update.

@bishtpawan
Copy link
Contributor

bishtpawan commented Oct 20, 2020

Hi @lcnr as you mentioned [find the place where we call Instance::resolve and put a tcx.normalize_erasing_regions before that] can you please elaborate it please? As I can see the Instace::resolve in [compiler/rustc_middle/src/ty/instance.rs] but don't get you point to put tcx.normalize_erasing_regions. Thanks

@lcnr
Copy link
Contributor

lcnr commented Oct 20, 2020

Can you post the resulting backtrace using RUST_BACKTRACE=1?

@bishtpawan
Copy link
Contributor

bishtpawan commented Oct 20, 2020

@lcnr, here it is:

0: rust_begin_unwind
   1: std::panicking::begin_panic_fmt
   2: rustc_trait_selection::traits::codegen::codegen_fulfill_obligation
   3: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::codegen_fulfill_obligation>::compute
   4: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
   5: rustc_data_structures::stack::ensure_sufficient_stack
   6: rustc_query_system::query::plumbing::get_query_impl
   7: rustc_ty::instance::inner_resolve_instance
   8: rustc_ty::instance::resolve_instance
   9: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::resolve_instance>::compute
  10: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  11: rustc_data_structures::stack::ensure_sufficient_stack
  12: rustc_query_system::query::plumbing::get_query_impl
  13: rustc_middle::ty::instance::Instance::resolve_opt_const_arg
  14: rustc_middle::ty::instance::Instance::resolve
  15: rustc_mir_build::lints::Search::is_recursive_call
  16: rustc_data_structures::graph::iterate::TriColorDepthFirstSearch<G>::run_from_start
  17: rustc_mir_build::lints::check
  18: rustc_infer::infer::InferCtxtBuilder::enter
  19: rustc_mir_build::build::mir_built
  20: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_built>::compute
  21: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  22: rustc_data_structures::stack::ensure_sufficient_stack
  23: rustc_query_system::query::plumbing::get_query_impl
  24: rustc_mir::transform::check_unsafety::unsafety_check_result
  25: core::ops::function::FnOnce::call_once
  26: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::unsafety_check_result>::compute
  27: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  28: rustc_data_structures::stack::ensure_sufficient_stack
  29: rustc_query_system::query::plumbing::get_query_impl
  30: rustc_query_system::query::plumbing::ensure_query_impl
  31: rustc_mir::transform::mir_const
  32: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_const>::compute
  33: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  34: rustc_data_structures::stack::ensure_sufficient_stack
  35: rustc_query_system::query::plumbing::get_query_impl
  36: rustc_mir::transform::mir_promoted
  37: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_promoted>::compute
  38: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  39: rustc_data_structures::stack::ensure_sufficient_stack
  40: rustc_query_system::query::plumbing::get_query_impl
  41: rustc_mir::borrow_check::mir_borrowck
  42: core::ops::function::FnOnce::call_once
  43: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_borrowck>::compute
  44: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  45: rustc_data_structures::stack::ensure_sufficient_stack
  46: rustc_query_system::query::plumbing::get_query_impl
  47: rustc_query_system::query::plumbing::ensure_query_impl
  48: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::par_body_owners
  49: rustc_interface::passes::analysis
  50: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  51: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  52: rustc_data_structures::stack::ensure_sufficient_stack
  53: rustc_query_system::query::plumbing::get_query_impl
  54: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  55: rustc_span::with_source_map
  56: rustc_interface::interface::create_compiler_and_run
  57: scoped_tls::ScopedKey<T>::set

@lcnr
Copy link
Contributor

lcnr commented Oct 20, 2020

So I think that the easiest fix is to add tcx.normalize_erasing_regions(param_env, substs) before this call.

let (callee, call_substs) =
if let Ok(Some(instance)) = Instance::resolve(tcx, param_env, callee, substs) {
(instance.def_id(), instance.substs)
} else {
(callee, substs)
};

I am however surprised that func.ty(body, tcx) isn't actually normalized here. cc @eddyb @nikomatsakis do we want to instead normalize this in mir building?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustfmt Area: Rustfmt C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants