exhaustiveness checker mishandles const reference to ADT in match pattern

[hcs64]

Update from @pnkfelix:

Our behavior on this test case (playpen) has changed, but it is still not correct.

#[derive(PartialEq, Eq)]
struct S;

fn main() {
    const C: &S = &S;
    match C {
        C => {}
    }
}

emits error:

error[E0004]: non-exhaustive patterns: `&S` not covered
 --> src/main.rs:6:11
  |
6 |     match C {
  |           ^ pattern `&S` not covered
  |
  = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

If you work-around the exhaustiveness checker bug by e.g. adding an unreachable arm, then the compiler accepts the code (and as far as I can tell, assigns it the correct runtime semantics).

Original bug report follows:

---

struct S;

fn main() {
    const C: &S = &S;
    match C {
        C => {}
    }
}

gives:
error: internal compiler error: librustc_mir/hair/pattern/_match.rs:432: bad constructor ConstantValue(Const { ty: &S, val: Scalar(Ptr(Pointer { alloc_id: AllocId(1), offset: Size { raw: 0 } })) }) for adt S

The relevant stack is:

  14: rustc_mir::hair::pattern::_match::Constructor::variant_index_for_adt
  15: rustc_mir::hair::pattern::_match::constructor_sub_pattern_tys
  16: rustc_mir::hair::pattern::_match::is_useful_specialized
  17: <core::iter::Map<I, F> as core::iter::iterator::Iterator>::try_fold
  18: rustc_mir::hair::pattern::_match::is_useful
  19: rustc_mir::hair::pattern::_match::is_useful_specialized
  20: <core::iter::Map<I, F> as core::iter::iterator::Iterator>::try_fold
  21: rustc_mir::hair::pattern::_match::is_useful
  22: rustc_mir::hair::pattern::check_match::check_arms
  23: rustc_mir::hair::pattern::_match::MatchCheckCtxt::create_and_enter
  24: <rustc_mir::hair::pattern::check_match::MatchVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_expr
  25: <rustc_mir::hair::pattern::check_match::MatchVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_expr
  26: <rustc_mir::hair::pattern::check_match::MatchVisitor<'a, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_body
  27: rustc::session::Session::track_errors
  28: rustc_mir::hair::pattern::check_match::check_match
  29: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::check_match<'tcx>>::compute
  30: rustc::dep_graph::graph::DepGraph::with_task_impl
  31: <rustc::ty::query::plumbing::JobOwner<'a, 'tcx, Q>>::start
  32: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  33: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  34: rustc::hir::intravisit::Visitor::visit_nested_body
  35: rustc::hir::Crate::visit_all_item_likes
  36: rustc_mir::hair::pattern::check_match::check_crate

It looks like this will take some effort to fix, I patched constructor_sub_pattern_tys to return vec![], but then rustc erroneously reports
error[E0004]: non-exhaustive patterns: '&S' not covered
and with an enum:

enum E {A}

fn main() {
    const C: &E = &E::A;
    match C {
        C => {}
        _ => {}
    }
}

it hits:
error: internal compiler error: librustc\traits\codegen\mod.rs:169: Encountered errors '[FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<E as std::cmp::PartialEq>)),depth=1),Unimplemented)]' resolving bounds after type-checking

[oli-obk]

cc @varkor you know this code very well. any pointers?

[varkor]

Okay, this is actually more interesting than first appears from the example: the matching code dealing with constants behind references is completely broken.

fn main() {
    const C: &bool = &true;
    let x = match C {
        C => "C",
        &true => "true",
        &false => "false",
    };
    println!("{}", x);
}

If you miss off the &true or &false branches, it won't compile, because it doesn't think the match is exhaustive, but if you run it, of course "C" is hit every time.
I think this will only fail when it is exhaustive, rather than thinking it's exhaustive when it's not, so I don't think it's a soundness bug, but something's going very wrong. I'll look into it.

[varkor]

I think the problem here is this:

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

Lines 209 to 221 in af2be23

	(&ty::Ref(_, rty, _), &PatternKind::Constant { ref value }) => {
	Pattern {
	ty: pat.ty,
	span: pat.span,
	kind: box PatternKind::Deref {
	subpattern: Pattern {
	ty: rty,
	span: pat.span,
	kind: box PatternKind::Constant { value: value.clone() },
	}
	}
	}
	}

When we encounter a constant, we try to turn it into a pattern, but the handling for references is wrong. We convert it into a deref pattern, but the underlying constant value it refers to still has type &T (and corresponding pointer value), rather than T.
I.e. the box PatternKind::Constant { value: value.clone() } line is incorrect, and should be referring to a dereferenced value instead.

@oli-obk: is there a way to dereference a ty::Const in this way?

[artemshein]

Just got it in my app. Is there a workaround or matching with constants is just broken for now?

[varkor]

@artemshein: I would try to avoid using constants in patterns right now; sorry – I haven't got around to fixing this yet.

[estebank]

So, there's something that can be done, which is calling report_fulfillment_errors which will in this case emit

error[E0277]: can't compare `S` with `S`
   |
   = help: the trait `std::cmp::PartialEq` is not implemented for `S`
   = note: required because of the requirements on the impl of `std::cmp::PartialEq` for `&S`

I do not know if this is correct though.

[oli-obk]

We don't want to report an error on this code. We should be accepting it as correct

[estebank]

@oli-obk I believe we should be accepting it as correct, but emitting the E0277 instead of an ICE can be a stop gap in the meantime to allow my current abort_if_errors removal effort to plow forward.

---

Edit: Scratch that, changing the code to be const C: S = S; causes the following error, which seems correct to me, right?

error: to use a constant of type `S` in a pattern, `S` must be annotated with `#[derive(PartialEq, Eq)]`
 --> ../../src/test/ui/consts/match_ice.rs:8:9
  |
8 |         C => {}
  |         ^

If that is the case, then the same error needs to be emitted for &S.

[estebank]

I think I figured how to correctly fix the ICE (by emitting the error in my previous comment) but the exhaustiveness check is still broken.

[oli-obk]

won't the ICE reoccur if you add the suggested derives?

[estebank]

In the linked commit I added a test for the struct with the derives and otherwise unchanged and we continue to get the uncovered patterns error, but otherwise things look fine.

[pnkfelix]

triage: P-medium.

[Nadrieril]

This appears to be fixed on nightly, probably due to #70743

[varkor]

(If there's already a test for this case, feel free to close again: I just want to be sure.)

[Nadrieril]

There wasn't. I just added one in #78072