Use precise errors during const to pat conversion instead of a catch-all on the main constant

Author: oli-obk

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

@@ -43,7 +43,7 @@ struct ConstToPat<'a, 'tcx> {
     span: Span,
     param_env: ty::ParamEnv<'tcx>,
 
-    // This tracks if we signal some hard error for a given const value, so that
+    // This tracks if we saw some error or lint for a given const value, so that
     // we will not subsequently issue an irrelevant lint for the same const
     // value.
     saw_const_match_error: Cell<bool>,
@@ -103,7 +103,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
         // once indirect_structural_match is a full fledged error, this
         // level of indirection can be eliminated
 
-        let inlined_const_as_pat = self.recur(cv);
+        let inlined_const_as_pat = self.recur(cv, mir_structural_match_violation);
 
         if self.include_lint_checks && !self.saw_const_match_error.get() {
             // If we were able to successfully convert the const to some pat,
@@ -216,7 +216,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
     }
 
     // Recursive helper for `to_pat`; invoke that (instead of calling this directly).
-    fn recur(&self, cv: &'tcx ty::Const<'tcx>) -> Pat<'tcx> {
+    fn recur(&self, cv: &'tcx ty::Const<'tcx>, mir_structural_match_violation: bool) -> Pat<'tcx> {
         let id = self.id;
         let span = self.span;
         let tcx = self.tcx();
@@ -227,7 +227,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 .enumerate()
                 .map(|(idx, val)| {
                     let field = Field::new(idx);
-                    FieldPat { field, pattern: self.recur(val) }
+                    FieldPat { field, pattern: self.recur(val, false) }
                 })
                 .collect()
         };
@@ -248,6 +248,21 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 tcx.sess.span_err(span, "cannot use unions in constant patterns");
                 PatKind::Wild
             }
+            ty::Adt(..)
+                if !self.type_has_partial_eq_impl(cv.ty)
+                    // FIXME(#73448): Find a way to bring const qualification into parity with
+                    // `search_for_structural_match_violation` and then remove this condition.
+                    && self.search_for_structural_match_violation(cv.ty).is_some() =>
+            {
+                let msg = format!(
+                    "to use a constant of type `{}` in a pattern, \
+                    `{}` must be annotated with `#[derive(PartialEq, Eq)]`",
+                    cv.ty, cv.ty,
+                );
+                self.saw_const_match_error.set(true);
+                self.tcx().sess.span_err(self.span, &msg);
+                PatKind::Wild
+            }
             // 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.
@@ -258,6 +273,20 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
             // 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() => {
+                if self.include_lint_checks && !self.saw_const_match_error.get() {
+                    self.saw_const_match_error.set(true);
+                    let msg = format!(
+                        "to use a constant of type `{}` in a pattern, \
+                        `{}` must be annotated with `#[derive(PartialEq, Eq)]`",
+                        cv.ty, cv.ty,
+                    );
+                    tcx.struct_span_lint_hir(
+                        lint::builtin::INDIRECT_STRUCTURAL_MATCH,
+                        id,
+                        span,
+                        |lint| lint.build(&msg).emit(),
+                    );
+                }
                 PatKind::Constant { value: cv }
             }
             ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => {
@@ -292,14 +321,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                     .destructure_const(param_env.and(cv))
                     .fields
                     .iter()
-                    .map(|val| self.recur(val))
+                    .map(|val| self.recur(val, false))
                     .collect(),
                 slice: None,
                 suffix: Vec::new(),
             },
             ty::Ref(_, pointee_ty, ..) => match *pointee_ty.kind() {
                 // These are not allowed and will error elsewhere anyway.
-                ty::Dynamic(..) => PatKind::Constant { value: cv },
+                ty::Dynamic(..) => {
+                    self.saw_const_match_error.set(true);
+                    tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty));
+                    PatKind::Wild
+                }
                 // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this
                 // optimization for now.
                 ty::Str => PatKind::Constant { value: cv },
@@ -321,7 +354,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                                     .destructure_const(param_env.and(array))
                                     .fields
                                     .iter()
-                                    .map(|val| self.recur(val))
+                                    .map(|val| self.recur(val, false))
                                     .collect(),
                                 slice: None,
                                 suffix: vec![],
@@ -333,16 +366,21 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                     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.
+                // Backwards compatibility hack: support references to non-structural types.
+                // We'll lower
+                // this pattern to a `PartialEq::eq` comparison and `PartialEq::eq` takes a
+                // reference. This makes the rest of the matching logic simpler as it doesn't have
+                // to figure out how to get a reference again.
                 ty::Adt(..) if !self.type_marked_structural(pointee_ty) => {
                     PatKind::Constant { value: cv }
                 }
+                // All other references are converted into deref patterns and then recursively
+                // convert the dereferenced constant to a pattern that is the sub-pattern of the
+                // deref pattern.
                 _ => {
                     let old = self.behind_reference.replace(true);
                     let val = PatKind::Deref {
-                        subpattern: self.recur(tcx.deref_const(self.param_env.and(cv))),
+                        subpattern: self.recur(tcx.deref_const(self.param_env.and(cv)), false),
                     };
                     self.behind_reference.set(old);
                     val
@@ -373,11 +411,34 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
                 PatKind::Constant { value: cv }
             }
             _ => {
-                tcx.sess.delay_span_bug(span, &format!("cannot make a pattern out of {}", cv.ty));
+                self.saw_const_match_error.set(true);
+                tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty));
                 PatKind::Wild
             }
         };
 
+        if self.include_lint_checks
+            && !self.saw_const_match_error.get()
+            && mir_structural_match_violation
+            // FIXME(#73448): Find a way to bring const qualification into parity with
+            // `search_for_structural_match_violation` and then remove this condition.
+            && self.search_for_structural_match_violation(cv.ty).is_some()
+        {
+            self.saw_const_match_error.set(true);
+            let msg = format!(
+                "to use a constant of type `{}` in a pattern, \
+                 the constant's initializer must be trivial or all types \
+                 in the constant must be annotated with `#[derive(PartialEq, Eq)]`",
+                cv.ty,
+            );
+            tcx.struct_span_lint_hir(
+                lint::builtin::NONTRIVIAL_STRUCTURAL_MATCH,
+                id,
+                span,
+                |lint| lint.build(&msg).emit(),
+            );
+        }
+
         Pat { span, ty: cv.ty, kind: Box::new(kind) }
     }
 }

compiler/rustc_session/src/lint/builtin.rs

@@ -2138,22 +2138,16 @@ declare_lint! {
     /// ```rust,compile_fail
     /// #![deny(indirect_structural_match)]
     ///
-    /// struct Plus(i32, i32);
-    /// const ONE_PLUS_TWO: &&Plus = &&Plus(1, 2);
-    ///
-    /// impl PartialEq for Plus {
-    ///     fn eq(&self, other: &Self) -> bool {
-    ///         self.0 + self.1 == other.0 + other.1
-    ///     }
-    /// }
-    ///
-    /// impl Eq for Plus {}
-    ///
+    /// struct NoDerive(i32);
+    /// impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } }
+    /// impl Eq for NoDerive { }
+    /// #[derive(PartialEq, Eq)]
+    /// struct WrapParam<T>(T);
+    /// const WRAP_INDIRECT_PARAM: & &WrapParam<NoDerive> = & &WrapParam(NoDerive(0));
     /// fn main() {
-    ///     if let ONE_PLUS_TWO = &&Plus(3, 0) {
-    ///         println!("semantic!");
-    ///     } else {
-    ///         println!("structural!");
+    ///     match WRAP_INDIRECT_PARAM {
+    ///         WRAP_INDIRECT_PARAM => { }
+    ///         _ => { }
     ///     }
     /// }
     /// ```
@@ -2170,9 +2164,8 @@ declare_lint! {
     /// [issue #62411]: https://github.com/rust-lang/rust/issues/62411
     /// [future-incompatible]: ../index.md#future-incompatible-lints
     pub INDIRECT_STRUCTURAL_MATCH,
-    // defaulting to allow until rust-lang/rust#62614 is fixed.
-    Allow,
-    "pattern with const indirectly referencing non-structural-match type",
+    Warn,
+    "constant used in pattern contains value of non-structural-match type in a field or a variant",
     @future_incompatible = FutureIncompatibleInfo {
         reference: "issue #62411 <https://github.com/rust-lang/rust/issues/62411>",
         edition: None,
@@ -2223,6 +2216,46 @@ declare_lint! {
     };
 }
 
+declare_lint! {
+    /// The `nontrivial_structural_match` lint detects constants that are used in patterns,
+    /// whose type is not structural-match and whose initializer body actually uses values
+    /// that are not structural-match. So `Option<NotStruturalMatch>` is ok if the constant
+    /// is just `None`.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(nontrivial_structural_match)]
+    ///
+    /// struct Plus(i32, i32);
+    /// const ONE_PLUS_TWO: &&Plus = &&Plus(1, 2);
+    ///
+    /// impl PartialEq for Plus {
+    ///     fn eq(&self, other: &Self) -> bool {
+    ///         self.0 + self.1 == other.0 + other.1
+    ///     }
+    /// }
+    ///
+    /// impl Eq for Plus {}
+    ///
+    /// fn main() {
+    ///     if let ONE_PLUS_TWO = &&Plus(3, 0) {
+    ///         println!("semantic!");
+    ///     } else {
+    ///         println!("structural!");
+    ///     }
+    /// }
+    /// ```
+    pub NONTRIVIAL_STRUCTURAL_MATCH,
+    Warn,
+    "constant used in pattern of non-structural-match type and the constant's initializer \
+    expression contains values of non-structural-match types",
+    @future_incompatible = FutureIncompatibleInfo {
+        reference: "issue #73448 <https://github.com/rust-lang/rust/issues/73448>",
+        edition: None,
+    };
+}
+
 declare_lint! {
     /// The `ambiguous_associated_items` lint detects ambiguity between
     /// [associated items] and [enum variants].
@@ -2657,6 +2690,7 @@ declare_lint_pass! {
         MUTABLE_BORROW_RESERVATION_CONFLICT,
         INDIRECT_STRUCTURAL_MATCH,
         POINTER_STRUCTURAL_MATCH,
+        NONTRIVIAL_STRUCTURAL_MATCH,
         SOFT_UNSTABLE,
         INLINE_NO_SANITIZE,
         ASM_SUB_REGISTER,

src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.rs

@@ -1,8 +1,5 @@
 // check-pass
 
-#![warn(indirect_structural_match)]
-//~^ NOTE lint level is defined here
-
 struct CustomEq;
 
 impl Eq for CustomEq {}
@@ -32,7 +29,8 @@ fn main() {
         BAR_BAZ => panic!(),
         //~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
         //~| WARN this was previously accepted
-        //~| NOTE see issue #62411
+        //~| NOTE see issue #73448
+        //~| NOTE `#[warn(nontrivial_structural_match)]` on by default
         _ => {}
     }
 }

src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr

@@ -1,16 +1,12 @@
-warning: to use a constant of type `CustomEq` in a pattern, `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]`
-  --> $DIR/custom-eq-branch-warn.rs:32:9
+warning: to use a constant of type `Foo` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]`
+  --> $DIR/custom-eq-branch-warn.rs:29:9
    |
 LL |         BAR_BAZ => panic!(),
    |         ^^^^^^^
    |
-note: the lint level is defined here
-  --> $DIR/custom-eq-branch-warn.rs:3:9
-   |
-LL | #![warn(indirect_structural_match)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: `#[warn(nontrivial_structural_match)]` on by default
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/62411>
+   = note: for more information, see issue #73448 <https://github.com/rust-lang/rust/issues/73448>
 
 warning: 1 warning emitted
 

src/test/ui/consts/const_in_pattern/issue-65466.rs

@@ -1,9 +1,7 @@
-// FIXME: This still ICEs.
-//
-// ignore-test
-
 #![deny(indirect_structural_match)]
 
+// check-pass
+
 #[derive(PartialEq, Eq)]
 enum O<T> {
     Some(*const T), // Can also use PhantomData<T>

src/test/ui/consts/const_in_pattern/issue-65466.stderr

@@ -27,6 +27,7 @@ fn main() {
     match None {
         NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
         //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]`
+        //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]`
         _ => panic!("whoops"),
     }
 }

src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs

@@ -1,8 +1,14 @@
-error: to use a constant of type `NoPartialEq` in a pattern, `NoPartialEq` must be annotated with `#[derive(PartialEq, Eq)]`
+error: to use a constant of type `Option<NoPartialEq>` in a pattern, `Option<NoPartialEq>` must be annotated with `#[derive(PartialEq, Eq)]`
   --> $DIR/reject_non_partial_eq.rs:28:9
    |
 LL |         NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
    |         ^^^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: to use a constant of type `Option<NoPartialEq>` in a pattern, `Option<NoPartialEq>` must be annotated with `#[derive(PartialEq, Eq)]`
+  --> $DIR/reject_non_partial_eq.rs:28:9
+   |
+LL |         NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
+   |         ^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors