Always fall back to PartialEq when a constant in a pattern is not recursively structural-eq

Author: oli-obk

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

@@ -380,7 +380,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         );
     }
 
-    /// Compare two `&T` values using `<T as std::compare::PartialEq>::eq`
+    /// Compare two values using `<T as std::compare::PartialEq>::eq`.
+    /// If the values are already references, just call it directly, otherwise
+    /// take a reference to the values first and then call it.
     fn non_scalar_compare(
         &mut self,
         block: BasicBlock,
@@ -441,12 +443,36 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
         }
 
-        let ty::Ref(_, deref_ty, _) = *ty.kind() else {
-            bug!("non_scalar_compare called on non-reference type: {}", ty);
-        };
+        match *ty.kind() {
+            ty::Ref(_, deref_ty, _) => ty = deref_ty,
+            _ => {
+                // non_scalar_compare called on non-reference type
+                let temp = self.temp(ty, source_info.span);
+                self.cfg.push_assign(block, source_info, temp, Rvalue::Use(expect));
+                let ref_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, ty);
+                let ref_temp = self.temp(ref_ty, source_info.span);
+
+                self.cfg.push_assign(
+                    block,
+                    source_info,
+                    ref_temp,
+                    Rvalue::Ref(self.tcx.lifetimes.re_erased, BorrowKind::Shared, temp),
+                );
+                expect = Operand::Move(ref_temp);
+
+                let ref_temp = self.temp(ref_ty, source_info.span);
+                self.cfg.push_assign(
+                    block,
+                    source_info,
+                    ref_temp,
+                    Rvalue::Ref(self.tcx.lifetimes.re_erased, BorrowKind::Shared, val),
+                );
+                val = ref_temp;
+            }
+        }
 
         let eq_def_id = self.tcx.require_lang_item(LangItem::PartialEq, Some(source_info.span));
-        let method = trait_method(self.tcx, eq_def_id, sym::eq, [deref_ty, deref_ty]);
+        let method = trait_method(self.tcx, eq_def_id, sym::eq, [ty, ty]);
 
         let bool_ty = self.tcx.types.bool;
         let eq_result = self.temp(bool_ty, source_info.span);

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

@@ -62,21 +62,13 @@ struct ConstToPat<'tcx> {
     treat_byte_string_as_slice: bool,
 }
 
-mod fallback_to_const_ref {
-    #[derive(Debug)]
-    /// This error type signals that we encountered a non-struct-eq situation behind a reference.
-    /// We bubble this up in order to get back to the reference destructuring and make that emit
-    /// a const pattern instead of a deref pattern. This allows us to simply call `PartialEq::eq`
-    /// on such patterns (since that function takes a reference) and not have to jump through any
-    /// hoops to get a reference to the value.
-    pub(super) struct FallbackToConstRef(());
-
-    pub(super) fn fallback_to_const_ref(c2p: &super::ConstToPat<'_>) -> FallbackToConstRef {
-        assert!(c2p.behind_reference.get());
-        FallbackToConstRef(())
-    }
-}
-use fallback_to_const_ref::{fallback_to_const_ref, FallbackToConstRef};
+/// This error type signals that we encountered a non-struct-eq situation.
+/// We bubble this up in order to get back to the reference destructuring and make that emit
+/// a const pattern instead of a deref pattern. This allows us to simply call `PartialEq::eq`
+/// on such patterns (since that function takes a reference) and not have to jump through any
+/// hoops to get a reference to the value.
+#[derive(Debug)]
+struct FallbackToConstRef;
 
 impl<'tcx> ConstToPat<'tcx> {
     fn new(
@@ -236,13 +228,13 @@ impl<'tcx> ConstToPat<'tcx> {
 
         let kind = match cv.ty().kind() {
             ty::Float(_) => {
-                    tcx.emit_spanned_lint(
-                        lint::builtin::ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
-                        id,
-                        span,
-                        FloatPattern,
-                    );
-                PatKind::Constant { value: cv }
+                tcx.emit_spanned_lint(
+                    lint::builtin::ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
+                    id,
+                    span,
+                    FloatPattern,
+                );
+                return Err(FallbackToConstRef);
             }
             ty::Adt(adt_def, _) if adt_def.is_union() => {
                 // Matching on union fields is unsafe, we can't hide it in constants
@@ -289,7 +281,7 @@ impl<'tcx> ConstToPat<'tcx> {
                 // Since we are behind a reference, we can just bubble the error up so we get a
                 // constant at reference type, making it easy to let the fallback call
                 // `PartialEq::eq` on it.
-                return Err(fallback_to_const_ref(self));
+                return Err(FallbackToConstRef);
             }
             ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty()) => {
                 debug!(
@@ -411,7 +403,7 @@ impl<'tcx> ConstToPat<'tcx> {
                                 IndirectStructuralMatch { non_sm_ty: *pointee_ty },
                             );
                         }
-                        PatKind::Constant { value: cv }
+                        return Err(FallbackToConstRef);
                     } else {
                         if !self.saw_const_match_error.get() {
                             self.saw_const_match_error.set(true);
@@ -439,20 +431,17 @@ impl<'tcx> ConstToPat<'tcx> {
                         // we fall back to a const pattern. If we do not do this, we may end up with
                         // a !structural-match constant that is not of reference type, which makes it
                         // very hard to invoke `PartialEq::eq` on it as a fallback.
-                        let val = match self.recur(tcx.deref_mir_constant(self.param_env.and(cv)), false) {
-                            Ok(subpattern) => PatKind::Deref { subpattern },
-                            Err(_) => PatKind::Constant { value: cv },
-                        };
+                        let subpattern = self.recur(tcx.deref_mir_constant(self.param_env.and(cv)), false)?;
                         self.behind_reference.set(old);
-                        val
+                        PatKind::Deref { subpattern }
                     }
                 }
             },
             ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::FnDef(..) => {
                 PatKind::Constant { value: cv }
             }
             ty::RawPtr(pointee) if pointee.ty.is_sized(tcx, param_env) => {
-                PatKind::Constant { value: cv }
+                return Err(FallbackToConstRef);
             }
             // FIXME: these can have very surprising behaviour where optimization levels or other
             // compilation choices change the runtime behaviour of the match.
@@ -469,7 +458,7 @@ impl<'tcx> ConstToPat<'tcx> {
                         PointerPattern
                     );
                 }
-                PatKind::Constant { value: cv }
+                return Err(FallbackToConstRef);
             }
             _ => {
                 self.saw_const_match_error.set(true);

tests/ui/pattern/usefulness/consts-opaque.rs

@@ -20,11 +20,12 @@ const BAR: Bar = Bar;
 #[derive(PartialEq)]
 enum Baz {
     Baz1,
-    Baz2
+    Baz2,
 }
 impl Eq for Baz {}
 const BAZ: Baz = Baz::Baz1;
 
+#[rustfmt::skip]
 fn main() {
     match FOO {
         FOO => {}
@@ -124,8 +125,16 @@ fn main() {
 
     match WRAPQUUX {
         Wrap(_) => {}
-        WRAPQUUX => {} // detected unreachable because we do inspect the `Wrap` layer
-        //~^ ERROR unreachable pattern
+        WRAPQUUX => {}
+    }
+
+    match WRAPQUUX {
+        Wrap(_) => {}
+    }
+
+    match WRAPQUUX {
+        //~^ ERROR: non-exhaustive patterns: `Wrap(_)` not covered
+        WRAPQUUX => {}
     }
 
     #[derive(PartialEq, Eq)]
@@ -138,8 +147,7 @@ fn main() {
     match WHOKNOWSQUUX {
         WHOKNOWSQUUX => {}
         WhoKnows::Yay(_) => {}
-        WHOKNOWSQUUX => {} // detected unreachable because we do inspect the `WhoKnows` layer
-        //~^ ERROR unreachable pattern
+        WHOKNOWSQUUX => {}
         WhoKnows::Nope => {}
     }
 }