ICE "Or-pattern should have been expanded earlier on"

[matthiaskrgr]

Code

This code is reduced from issue-54538-unused-parens-lint.rs

#![feature(or_patterns)]

fn main() {
    if let 0 | (1 | 2) = 0 {}
}

Meta

rustc --version --verbose:

rustc 1.43.0-nightly (564758c4c 2020-03-08)
binary: rustc
commit-hash: 564758c4c329e89722454dd2fbb35f1ac0b8b47c
commit-date: 2020-03-08
host: x86_64-unknown-linux-gnu
release: 1.43.0-nightly
LLVM version: 9.0

clippy 0.0.212 (329923e 2020-03-04)

Error output

warning: unnecessary parentheses around pattern
 --> ./ui/lint/issue-54538-unused-parens-lint.rs:3:16
  |
3 |     if let 0 | (1 | 2) = 0 {}
  |                ^^^^^^^ help: remove these parentheses
  |
  = note: `#[warn(unused_parens)]` on by default

error: internal compiler error: src/librustc_mir_build/hair/pattern/_match.rs:1884: Or-pattern should have been expanded earlier on.

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:875:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1063
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1428
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:204
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:224
  10: clippy_driver::report_clippy_ice
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:474
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::bug
  14: rustc_errors::Handler::bug
  15: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc::ty::context::tls::with_opt::{{closure}}
  17: rustc::ty::context::tls::with_opt
  18: rustc::util::bug::opt_span_bug_fmt
  19: rustc::util::bug::bug_fmt
  20: rustc_mir_build::hair::pattern::_match::pat_constructor
  21: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter
  22: rustc_mir_build::hair::pattern::_match::is_useful
  23: rustc_mir_build::hair::pattern::check_match::check_arms
  24: rustc_mir_build::hair::pattern::_match::MatchCheckCtxt::create_and_enter
  25: <rustc_mir_build::hair::pattern::check_match::MatchVisitor as rustc_hir::intravisit::Visitor>::visit_expr
  26: <rustc_mir_build::hair::pattern::check_match::MatchVisitor as rustc_hir::intravisit::Visitor>::visit_expr
  27: rustc_mir_build::hair::pattern::check_match::check_match
  28: rustc::ty::query::__query_compute::check_match
  29: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::check_match>::compute
  30: rustc::dep_graph::graph::DepGraph::with_task_impl
  31: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  32: rustc::ty::<impl rustc::ty::context::TyCtxt>::par_body_owners
  33: std::panicking::try::do_call
  34: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:86
  35: rustc_session::utils::<impl rustc_session::session::Session>::time
  36: rustc_interface::passes::analysis
  37: rustc::ty::query::__query_compute::analysis
  38: rustc::dep_graph::graph::DepGraph::with_task_impl
  39: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  40: rustc::ty::context::tls::enter_global
  41: rustc_interface::interface::run_compiler_in_existing_thread_pool
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new

note: Clippy version: clippy 0.0.212 (329923e 2020-03-04)

query stack during panic:
#0 [check_match] processing `main`
#1 [analysis] running analysis passes on this crate
end of query stack
error: aborting due to previous error

[Centril]

Another variation on this to include in a regression test:

#![feature(or_patterns)]

fn main() {
    if let x @ 0 | x @ (1 | 2) = 0 {}
}

[Centril]

cc @varkor @Nadrieril

[Nadrieril]

Ah yeah, I see the issue. There's an implicit invariant that there should be no or-patterns directly in the first column of the matrix, but this invariant is broken exactly when an or-pattern has a child that is itself an or-pattern. I should have documented that.
Specifically this fails to preserve the invariant:

rust/src/librustc_mir_build/hair/pattern/_match.rs

Lines 480 to 487 in dd155df

	/// Pushes a new row to the matrix. If the row starts with an or-pattern, this expands it.
	crate fn push(&mut self, row: PatStack<'p, 'tcx>) {
	if let Some(rows) = row.expand_or_pat() {
	self.0.extend(rows);
	} else {
	self.0.push(row);
	}
	}

I don't expect to have the bandwidth to work on rustc in the foreseeable future sadly. I anyone wants to try, I believe the fix could be as simple as replacing in the above function self.0.extend(rows) with a loop that calls self.push (and not self.0.push). That will ensure that we recursively expand or-patterns as needed instead of just expanding the first one.

[Centril]

@Nadrieril Yep, that did it. Thanks for the pointer. This issue is fixed by #69891.