Implement destructuring for all aggregates and for references

Author: oli-obk

compiler/rustc_mir/src/const_eval/mod.rs

@@ -93,8 +93,7 @@ pub(crate) fn deref_const<'tcx>(
         MemPlaceMeta::None => mplace.layout.ty,
         MemPlaceMeta::Poison => bug!("poison metadata in `deref_const`: {:#?}", mplace),
         // In case of unsized types, figure out the real type behind.
-        MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind {
-            ty::Dynamic(..) => ecx.read_drop_type_from_vtable(scalar).unwrap().1,
+        MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() {
             ty::Str => bug!("there's no sized equivalent of a `str`"),
             ty::Slice(elem_ty) => tcx.mk_array(elem_ty, scalar.to_machine_usize(&tcx).unwrap()),
             _ => bug!(

compiler/rustc_mir_build/src/build/matches/test.rs

@@ -409,7 +409,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         let deref_ty = match *ty.kind() {
             ty::Ref(_, deref_ty, _) => deref_ty,
-            _ => bug!("non_scalar_compare called on non-reference type: {}", ty),
+            _ => {
+                trace!("non_scalar_compare called on non-reference type: {}", ty);
+                // Backcompat hack: due to non-structural matches not being a hard error, we can
+                // reach this for types that have manual `Eq` or `PartialEq` impls.
+                assert!(!ty.is_structural_eq_shallow(self.hir.tcx()));
+                let ref_ty = self.hir.tcx().mk_imm_ref(self.hir.tcx().lifetimes.re_erased, ty);
+                // let y = &place;
+                let y = self.temp(ref_ty, source_info.span);
+                self.cfg.push_assign(
+                    block,
+                    source_info,
+                    y,
+                    Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, place),
+                );
+                val = Operand::Move(y);
+                // let temp = expect;
+                let temp = self.temp(ty, source_info.span);
+                self.cfg.push_assign(
+                    block,
+                    source_info,
+                    temp,
+                    Rvalue::Use(expect),
+                );
+                // reftemp = &temp;
+                let reftemp = self.temp(ref_ty, source_info.span);
+                self.cfg.push_assign(
+                    block,
+                    source_info,
+                    reftemp,
+                    Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, temp),
+                );
+                expect = Operand::Move(reftemp);
+                ty
+            },
         };
 
         let eq_def_id = self.hir.tcx().require_lang_item(LangItem::PartialEq, None);

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

@@ -28,10 +28,13 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
         debug!("const_to_pat: cv={:#?} id={:?}", cv, id);
         debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);
 
-        self.tcx.infer_ctxt().enter(|infcx| {
+        let pat = self.tcx.infer_ctxt().enter(|infcx| {
             let mut convert = ConstToPat::new(self, id, span, infcx);
             convert.to_pat(cv, mir_structural_match_violation)
-        })
+        });
+
+        debug!("const_to_pat: pat={:?}", pat);
+        pat
     }
 }
 
@@ -45,6 +48,10 @@ struct ConstToPat<'a, 'tcx> {
     // value.
     saw_const_match_error: Cell<bool>,
 
+    // For backcompat we need to keep allowing non-structurally-eq types behind references.
+    // See also all the `cant-hide-behind` tests.
+    behind_reference: Cell<bool>,
+
     // inference context used for checking `T: Structural` bounds.
     infcx: InferCtxt<'a, 'tcx>,
 
@@ -65,6 +72,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
             param_env: pat_ctxt.param_env,
             include_lint_checks: pat_ctxt.include_lint_checks,
             saw_const_match_error: Cell::new(false),
+            behind_reference: Cell::new(false),
         }
     }
 
@@ -233,7 +241,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 tcx.sess.span_err(span, "cannot use unions in constant patterns");
                 PatKind::Wild
             }
-            // keep old code until future-compat upgraded to errors.
+            // If the type is not structurally comparable, just emit the constant directly,
+            // causing the pattern match code to treat it opaquely.
+            // FIXME: This code doesn't emit errors itself, the caller emits the errors.
+            // So instead of specific errors, you just get blanket errors about the whole
+            // const type. See
+            // https://github.com/rust-lang/rust/pull/70743#discussion_r404701963 for
+            // details.
+            // Backwards compatibility hack because we can't cause hard errors on these
+            // types, so we compare them via `PartialEq::eq` at runtime.
+            ty::Adt(..) if !self.type_marked_structural(cv.ty) && self.behind_reference.get() => {
+                PatKind::Constant { value: cv }
+            }
             ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => {
                 debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, cv.ty);
                 let path = tcx.def_path_str(adt_def.did);
@@ -246,28 +265,6 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 tcx.sess.span_err(span, &msg);
                 PatKind::Wild
             }
-            // keep old code until future-compat upgraded to errors.
-            ty::Ref(_, adt_ty, _) if adt_ty.is_adt() && !self.type_marked_structural(adt_ty) => {
-                let adt_def =
-                    if let ty::Adt(adt_def, _) = adt_ty.kind() { adt_def } else { unreachable!() };
-
-                debug!(
-                    "adt_def {:?} has !type_marked_structural for adt_ty: {:?}",
-                    adt_def, adt_ty
-                );
-
-                // HACK(estebank): Side-step ICE #53708, but anything other than erroring here
-                // would be wrong. Returnging `PatKind::Wild` is not technically correct.
-                let path = tcx.def_path_str(adt_def.did);
-                let msg = format!(
-                    "to use a constant of type `{}` in a pattern, \
-                     `{}` must be annotated with `#[derive(PartialEq, Eq)]`",
-                    path, path,
-                );
-                self.saw_const_match_error.set(true);
-                tcx.sess.span_err(span, &msg);
-                PatKind::Wild
-            }
             ty::Adt(adt_def, substs) if adt_def.is_enum() => {
                 let destructured = tcx.destructure_const(param_env.and(cv));
                 PatKind::Variant {
@@ -293,7 +290,68 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 slice: None,
                 suffix: Vec::new(),
             },
-            _ => PatKind::Constant { value: cv },
+            ty::Ref(_, pointee_ty, ..) => match *pointee_ty.kind() {
+                // These are not allowed and will error elsewhere anyway.
+                ty::Dynamic(..) => PatKind::Constant { value: cv },
+                // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this
+                // optimization for now.
+                ty::Str => PatKind::Constant { value: cv },
+                ty::Slice(elem_ty) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv },
+                // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when
+                // matching against references, you can only use byte string literals.
+                // FIXME: clean this up, likely by permitting array patterns when matching on slices
+                ty::Array(elem_ty, _) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv },
+                // Cannot merge this with the catch all branch below, because the `const_deref`
+                // changes the type from slice to array, and slice patterns behave differently from
+                // array patterns.
+                ty::Slice(..) => {
+                    let old = self.behind_reference.replace(true);
+                    let array = tcx.deref_const(self.param_env.and(cv));
+                    let val = PatKind::Deref {
+                        subpattern: Pat {
+                            kind: Box::new(PatKind::Slice {
+                                prefix: tcx
+                                    .destructure_const(param_env.and(array))
+                                    .fields
+                                    .iter()
+                                    .map(|val| self.recur(val))
+                                    .collect(),
+                                slice: None,
+                                suffix: vec![],
+                            }),
+                            span,
+                            ty: pointee_ty,
+                        },
+                    };
+                    self.behind_reference.set(old);
+                    val
+                }
+                // Backwards compatibility hack. Don't take away the reference, since
+                // `PartialEq::eq` takes a reference, this makes the rest of the matching logic
+                // simpler.
+                ty::Adt(..) if !self.type_marked_structural(pointee_ty) => {
+                    PatKind::Constant { value: cv }
+                }
+                _ => {
+                    let old = self.behind_reference.replace(true);
+                    let val = PatKind::Deref {
+                        subpattern: self.recur(tcx.deref_const(self.param_env.and(cv))),
+                    };
+                    self.behind_reference.set(old);
+                    val
+                }
+            },
+            ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::FnDef(..) => {
+                PatKind::Constant { value: cv }
+            }
+            // FIXME: these can have very suprising behaviour where optimization levels or other
+            // compilation choices change the runtime behaviour of the match.
+            // See https://github.com/rust-lang/rust/issues/70861 for examples.
+            ty::FnPtr(..) | ty::RawPtr(..) => PatKind::Constant { value: cv },
+            _ => {
+                tcx.sess.delay_span_bug(span, &format!("cannot make a pattern out of {}", cv.ty));
+                PatKind::Wild
+            }
         };
 
         Pat { span, ty: cv.ty, kind: Box::new(kind) }

library/std/src/ffi/os_str.rs

@@ -94,7 +94,7 @@ pub struct OsString {
 // `OsStr::from_inner` current implementation relies
 // on `OsStr` being layout-compatible with `Slice`.
 // When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`.
-// Anyway, `OsStr` representation and layout are considered implementation detail, are
+// Anyway, `OsStr` representation and layout are considered implementation details, are
 // not documented and must not be relied upon.
 pub struct OsStr {
     inner: Slice,

src/test/ui/consts/consts-in-patterns.rs

@@ -19,8 +19,6 @@ pub fn main() {
     assert_eq!(y, 2);
     let z = match &() {
         ZST => 9,
-        // FIXME: this should not be required
-        _ => 42,
     };
     assert_eq!(z, 9);
     let z = match b"" {

src/test/ui/consts/match_ice.rs

@@ -10,10 +10,9 @@ fn main() {
     match C {
         C => {}
         //~^ ERROR to use a constant of type `S` in a pattern, `S` must be annotated with
-        //~| ERROR to use a constant of type `S` in a pattern, `S` must be annotated with
     }
     const K: &T = &T;
-    match K { //~ ERROR non-exhaustive patterns: `&T` not covered
+    match K {
         K => {}
     }
 }

src/test/ui/consts/match_ice.stderr

@@ -4,24 +4,5 @@ error: to use a constant of type `S` in a pattern, `S` must be annotated with `#
 LL |         C => {}
    |         ^
 
-error[E0004]: non-exhaustive patterns: `&T` not covered
-  --> $DIR/match_ice.rs:16:11
-   |
-LL | struct T;
-   | --------- `T` defined here
-...
-LL |     match K {
-   |           ^ pattern `&T` not covered
-   |
-   = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
-   = note: the matched value is of type `&T`
-
-error: to use a constant of type `S` in a pattern, `S` must be annotated with `#[derive(PartialEq, Eq)]`
-  --> $DIR/match_ice.rs:11:9
-   |
-LL |         C => {}
-   |         ^
-
-error: aborting due to 3 previous errors
+error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0004`.

src/test/ui/match/pattern-deref-miscompile.rs

@@ -0,0 +1,46 @@
+// run-pass
+
+fn main() {
+    match b"." as &[u8] {
+        b"." if true => {},
+        b"." => panic!(),
+        b".." => panic!(),
+        b"" => panic!(),
+        _ => panic!(),
+    }
+    match b"." as &[u8] {
+        b"." if false => panic!(),
+        b"." => {},
+        b".." => panic!(),
+        b"" => panic!(),
+        _ => panic!(),
+    }
+    match b".." as &[u8] {
+        b"." if true => panic!(), // the miscompile caused this arm to be reached
+        b"." => panic!(),
+        b".." => {},
+        b"" => panic!(),
+        _ => panic!(),
+    }
+    match b".." as &[u8] {
+        b"." if false => panic!(),
+        b"." => panic!(),
+        b".." => {},
+        b"" => panic!(),
+        _ => panic!(),
+    }
+    match b"" as &[u8] {
+        b"." if true => panic!(),
+        b"." => panic!(),
+        b".." => panic!(),
+        b"" => {},
+        _ => panic!(),
+    }
+    match b"" as &[u8] {
+        b"." if false => panic!(),
+        b"." => panic!(),
+        b".." => panic!(),
+        b"" => {},
+        _ => panic!(),
+    }
+}

src/test/ui/pattern/const-pat-ice.rs

@@ -1,10 +1,4 @@
-// failure-status: 101
-// rustc-env:RUST_BACKTRACE=0
-// normalize-stderr-test "note: rustc 1.* running on .*" -> "note: rustc VERSION running on TARGET"
-// normalize-stderr-test "note: compiler flags: .*" -> "note: compiler flags: FLAGS"
-// normalize-stderr-test "/_match.rs:[0-9]+:[0-9]+" -> "/_match.rs:LL:CC"
-
-// This is a repro test for an ICE in our pattern handling of constants.
+// check-pass
 
 const FOO: &&&u32 = &&&42;