match ergonomics for string and byte string literal patterns

Author: dianne

compiler/rustc_hir_typeck/src/pat.rs

@@ -177,16 +177,20 @@ enum PeelKind {
     /// Only peel reference types. This is used for explicit `deref!(_)` patterns, which dereference
     /// any number of `&`/`&mut` references, plus a single smart pointer.
     ExplicitDerefPat,
-    /// Implicitly peel any number of references, and if `deref_patterns` is enabled, smart pointer
-    /// ADTs. In order to peel only as much as necessary for the pattern to match, the `until_adt`
-    /// field contains the ADT def that the pattern is a constructor for, if applicable, so that we
-    /// don't peel it. See [`ResolvedPat`] for more information.
-    Implicit { until_adt: Option<DefId> },
+    /// Implicitly peel references, and if `deref_patterns` is enabled, smart pointer ADTs.
+    Implicit {
+        /// The ADT the pattern is a constructor for, if applicable, so that we don't peel it. See
+        /// [`ResolvedPat`] for more information.
+        until_adt: Option<DefId>,
+        /// The number of references at the head of the pattern's type, so we can leave that many
+        /// untouched. This is `1` for string literals, and `0` for most patterns.
+        pat_ref_layers: usize,
+    },
 }
 
 impl AdjustMode {
     const fn peel_until_adt(opt_adt_def: Option<DefId>) -> AdjustMode {
-        AdjustMode::Peel { kind: PeelKind::Implicit { until_adt: opt_adt_def } }
+        AdjustMode::Peel { kind: PeelKind::Implicit { until_adt: opt_adt_def, pat_ref_layers: 0 } }
     }
     const fn peel_all() -> AdjustMode {
         AdjustMode::peel_until_adt(None)
@@ -488,9 +492,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         match pat.kind {
             // Peel off a `&` or `&mut` from the scrutinee type. See the examples in
             // `tests/ui/rfcs/rfc-2005-default-binding-mode`.
-            _ if let AdjustMode::Peel { .. } = adjust_mode
+            _ if let AdjustMode::Peel { kind: peel_kind } = adjust_mode
                 && pat.default_binding_modes
-                && let ty::Ref(_, inner_ty, inner_mutability) = *expected.kind() =>
+                && let ty::Ref(_, inner_ty, inner_mutability) = *expected.kind()
+                && self.should_peel_ref(peel_kind, expected) =>
             {
                 debug!("inspecting {:?}", expected);
 
@@ -665,21 +670,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
             // String and byte-string literals result in types `&str` and `&[u8]` respectively.
             // All other literals result in non-reference types.
-            // As a result, we allow `if let 0 = &&0 {}` but not `if let "foo" = &&"foo" {}`.
-            //
-            // Call `resolve_vars_if_possible` here for inline const blocks.
-            PatKind::Expr(lt) => match self.resolve_vars_if_possible(self.check_pat_expr_unadjusted(lt)).kind() {
-                ty::Ref(..) => AdjustMode::Pass,
-                _ => {
-                    // Path patterns have already been handled, and inline const blocks currently
-                    // aren't possible to write, so any handling for them would be untested.
-                    if cfg!(debug_assertions)
-                        && self.tcx.features().deref_patterns()
-                        && !matches!(lt.kind, PatExprKind::Lit { .. })
-                    {
-                        span_bug!(lt.span, "FIXME(deref_patterns): adjust mode unimplemented for {:?}", lt.kind);
+            // As a result, we allow `if let 0 = &&0 {}` but not `if let "foo" = &&"foo" {}` unless
+            // `deref_patterns` is enabled.
+            PatKind::Expr(lt) => {
+                // Path patterns have already been handled, and inline const blocks currently
+                // aren't possible to write, so any handling for them would be untested.
+                if cfg!(debug_assertions)
+                    && self.tcx.features().deref_patterns()
+                    && !matches!(lt.kind, PatExprKind::Lit { .. })
+                {
+                    span_bug!(lt.span, "FIXME(deref_patterns): adjust mode unimplemented for {:?}", lt.kind);
+                }
+                // Call `resolve_vars_if_possible` here for inline const blocks.
+                let lit_ty = self.resolve_vars_if_possible(self.check_pat_expr_unadjusted(lt));
+                // If `deref_patterns` is enabled, allow `if let "foo" = &&"foo" {}`.
+                if self.tcx.features().deref_patterns() {
+                    let mut peeled_ty = lit_ty;
+                    let mut pat_ref_layers = 0;
+                    while let ty::Ref(_, inner_ty, mutbl) = *peeled_ty.kind() {
+                        // We rely on references at the head of constants being immutable.
+                        debug_assert!(mutbl.is_not());
+                        pat_ref_layers += 1;
+                        peeled_ty = inner_ty;
                     }
-                    AdjustMode::peel_all()
+                    AdjustMode::Peel { kind: PeelKind::Implicit { until_adt: None, pat_ref_layers } }
+                } else {
+                    if lit_ty.is_ref() { AdjustMode::Pass } else { AdjustMode::peel_all() }
                 }
             },
 
@@ -705,6 +721,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    /// Assuming `expected` is a reference type, determine whether to peel it before matching.
+    fn should_peel_ref(&self, peel_kind: PeelKind, mut expected: Ty<'tcx>) -> bool {
+        debug_assert!(expected.is_ref());
+        let pat_ref_layers = match peel_kind {
+            PeelKind::ExplicitDerefPat => 0,
+            PeelKind::Implicit { pat_ref_layers, .. } => pat_ref_layers,
+        };
+
+        // Most patterns don't have reference types, so we'll want to peel all references from the
+        // scrutinee before matching. To optimize for the common case, return early.
+        if pat_ref_layers == 0 {
+            return true;
+        }
+        debug_assert!(
+            self.tcx.features().deref_patterns(),
+            "Peeling for patterns with reference types is gated by `deref_patterns`."
+        );
+
+        // If the pattern has as many or more layers of reference as the expected type, we can match
+        // without peeling more, *unless* we find a smart pointer that we also need to peel.
+        // TODO: always peel `&mut`
+        let mut expected_ref_layers = 0;
+        while let ty::Ref(_, inner_ty, _) = *expected.kind() {
+            expected_ref_layers += 1;
+            expected = inner_ty;
+        }
+        pat_ref_layers < expected_ref_layers || self.should_peel_smart_pointer(peel_kind, expected)
+    }
+
     /// Determine whether `expected` is a smart pointer type that should be peeled before matching.
     fn should_peel_smart_pointer(&self, peel_kind: PeelKind, expected: Ty<'tcx>) -> bool {
         // Explicit `deref!(_)` patterns match against smart pointers; don't peel in that case.

tests/ui/pattern/deref-patterns/const-pats-do-not-mislead-inference.rs

@@ -0,0 +1,48 @@
+//@ revisions: stable deref_patterns
+//@[deref_patterns] check-pass
+//! `deref_patterns` allows string and byte string literal patterns to implicitly peel references
+//! and smart pointers from the scrutinee before matching. Since strings and byte strings themselves
+//! have reference types, we need to make sure we don't peel too much. By leaving the type of the
+//! match scrutinee partially uninferred, these tests make sure we only peel as much as needed in
+//! order to match. In particular, when peeling isn't needed, the results should be the same was
+//! we'd get without `deref_patterns` enabled.
+
+#![cfg_attr(deref_patterns, feature(deref_patterns))]
+#![cfg_attr(deref_patterns, expect(incomplete_features))]
+
+fn uninferred<T>() -> T { unimplemented!() }
+
+// Assert type equality without allowing coercions.
+trait Is<T> {}
+impl<T> Is<T> for T {}
+fn has_type<T>(_: impl Is<T>) {}
+
+fn main() {
+    // We don't need to peel anything to unify the type of `x` with `&str`, so `x: &str`.
+    let x = uninferred();
+    if let "..." = x {}
+    has_type::<&str>(x);
+
+    // We don't need to peel anything to unify the type of `&x` with `&[u8; 3]`, so `x: [u8; 3]`.
+    let x = uninferred();
+    if let b"..." = &x {}
+    has_type::<[u8; 3]>(x);
+
+    // Peeling a single `&` lets us unify the type of `&x` with `&[u8; 3]`, giving `x: [u8; 3]`.
+    let x = uninferred();
+    if let b"..." = &&x {}
+    //[stable]~^ ERROR: mismatched types
+    has_type::<[u8; 3]>(x);
+
+    // We have to peel both the `&` and the box before unifying the type of `x` with `&str`.
+    let x = uninferred();
+    if let "..." = &Box::new(x) {}
+    //[stable]~^ ERROR mismatched types
+    has_type::<&str>(x);
+
+    // After peeling the box, we can unify the type of `&x` with `&[u8; 3]`, giving `x: [u8; 3]`.
+    let x = uninferred();
+    if let b"..." = Box::new(&x) {}
+    //[stable]~^ ERROR mismatched types
+    has_type::<[u8; 3]>(x);
+}

tests/ui/pattern/deref-patterns/const-pats-do-not-mislead-inference.stable.stderr

@@ -0,0 +1,44 @@
+error[E0308]: mismatched types
+  --> $DIR/const-pats-do-not-mislead-inference.rs:33:12
+   |
+LL |     if let b"..." = &&x {}
+   |            ^^^^^^   --- this expression has type `&&_`
+   |            |
+   |            expected `&&_`, found `&[u8; 3]`
+   |
+   = note: expected reference `&&_`
+              found reference `&'static [u8; 3]`
+
+error[E0308]: mismatched types
+  --> $DIR/const-pats-do-not-mislead-inference.rs:39:12
+   |
+LL |     if let "..." = &Box::new(x) {}
+   |            ^^^^^   ------------ this expression has type `&Box<_>`
+   |            |
+   |            expected `&Box<_>`, found `&str`
+   |
+   = note: expected reference `&Box<_>`
+              found reference `&'static str`
+help: consider dereferencing to access the inner value using the Deref trait
+   |
+LL |     if let "..." = &*Box::new(x) {}
+   |                     +
+
+error[E0308]: mismatched types
+  --> $DIR/const-pats-do-not-mislead-inference.rs:45:12
+   |
+LL |     if let b"..." = Box::new(&x) {}
+   |            ^^^^^^   ------------ this expression has type `Box<&_>`
+   |            |
+   |            expected `Box<&_>`, found `&[u8; 3]`
+   |
+   = note: expected struct `Box<&_>`
+           found reference `&'static [u8; 3]`
+help: consider dereferencing to access the inner value using the Deref trait
+   |
+LL |     if let b"..." = *Box::new(&x) {}
+   |                     +
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0308`.

tests/ui/pattern/deref-patterns/needs-gate.rs

@@ -29,4 +29,21 @@ fn main() {
         //~^ ERROR: mismatched types
         _ => {}
     }
+
+    // `deref_patterns` allows string and byte string patterns to implicitly peel references.
+    match &"str" {
+        "str" => {}
+        //~^ ERROR: mismatched types
+        _ => {}
+    }
+    match &b"str" {
+        b"str" => {}
+        //~^ ERROR: mismatched types
+        _ => {}
+    }
+    match "str".to_owned() {
+        "str" => {}
+        //~^ ERROR: mismatched types
+        _ => {}
+    }
 }

tests/ui/pattern/deref-patterns/needs-gate.stderr

@@ -47,7 +47,37 @@ LL |     match *(b"test" as &[u8]) {
 LL |         b"test" => {}
    |         ^^^^^^^ expected `[u8]`, found `&[u8; 4]`
 
-error: aborting due to 5 previous errors
+error[E0308]: mismatched types
+  --> $DIR/needs-gate.rs:35:9
+   |
+LL |     match &"str" {
+   |           ------ this expression has type `&&str`
+LL |         "str" => {}
+   |         ^^^^^ expected `&&str`, found `&str`
+   |
+   = note: expected reference `&&_`
+              found reference `&'static _`
+
+error[E0308]: mismatched types
+  --> $DIR/needs-gate.rs:40:9
+   |
+LL |     match &b"str" {
+   |           ------- this expression has type `&&[u8; 3]`
+LL |         b"str" => {}
+   |         ^^^^^^ expected `&&[u8; 3]`, found `&[u8; 3]`
+   |
+   = note: expected reference `&&_`
+              found reference `&'static _`
+
+error[E0308]: mismatched types
+  --> $DIR/needs-gate.rs:45:9
+   |
+LL |     match "str".to_owned() {
+   |           ---------------- this expression has type `String`
+LL |         "str" => {}
+   |         ^^^^^ expected `String`, found `&str`
+
+error: aborting due to 8 previous errors
 
 Some errors have detailed explanations: E0308, E0658.
 For more information about an error, try `rustc --explain E0308`.

tests/ui/pattern/deref-patterns/strings.rs

@@ -14,10 +14,18 @@ fn main() {
         };
         assert_eq!(test_actual, test_expect);
 
-        // Test string literals in explicit `deref!(_)` patterns.
+        // Test string literals in deref patterns.
         let test_actual = match test_in.to_string() {
             deref!("zero") => 0,
-            deref!("one") => 1,
+            "one" => 1,
+            _ => 2,
+        };
+        assert_eq!(test_actual, test_expect);
+
+        // Test peeling references in addition to smart pointers.
+        let test_actual = match &test_in.to_string() {
+            deref!("zero") => 0,
+            "one" => 1,
             _ => 2,
         };
         assert_eq!(test_actual, test_expect);
@@ -47,18 +55,18 @@ fn main() {
         };
         assert_eq!(test_actual, test_expect);
 
-        // Test byte string literals used as arrays in explicit `deref!(_)` patterns.
+        // Test byte string literals used as arrays in deref patterns.
         let test_actual = match Box::new(*test_in) {
             deref!(b"0") => 0,
-            deref!(b"1") => 1,
+            b"1" => 1,
             _ => 2,
         };
         assert_eq!(test_actual, test_expect);
 
-        // Test byte string literals used as slices in explicit `deref!(_)` patterns.
+        // Test byte string literals used as slices in deref patterns.
         let test_actual = match test_in.to_vec() {
             deref!(b"0") => 0,
-            deref!(b"1") => 1,
+            b"1" => 1,
             _ => 2,
         };
         assert_eq!(test_actual, test_expect);

tests/ui/pattern/deref-patterns/typeck_fail.rs

@@ -2,19 +2,6 @@
 #![allow(incomplete_features)]
 
 fn main() {
-    // FIXME(deref_patterns): fails to typecheck because string literal patterns don't peel
-    // references from the scrutinee.
-    match "foo".to_string() {
-        "foo" => {}
-        //~^ ERROR: mismatched types
-        _ => {}
-    }
-    match &"foo".to_string() {
-        "foo" => {}
-        //~^ ERROR: mismatched types
-        _ => {}
-    }
-
     // Make sure we don't try implicitly dereferncing any ADT.
     match Some(0) {
         Ok(0) => {}

tests/ui/pattern/deref-patterns/typeck_fail.stderr

@@ -1,24 +1,5 @@
 error[E0308]: mismatched types
-  --> $DIR/typeck_fail.rs:8:9
-   |
-LL |     match "foo".to_string() {
-   |           ----------------- this expression has type `String`
-LL |         "foo" => {}
-   |         ^^^^^ expected `String`, found `&str`
-
-error[E0308]: mismatched types
-  --> $DIR/typeck_fail.rs:13:9
-   |
-LL |     match &"foo".to_string() {
-   |           ------------------ this expression has type `&String`
-LL |         "foo" => {}
-   |         ^^^^^ expected `&String`, found `&str`
-   |
-   = note: expected reference `&String`
-              found reference `&'static str`
-
-error[E0308]: mismatched types
-  --> $DIR/typeck_fail.rs:20:9
+  --> $DIR/typeck_fail.rs:7:9
    |
 LL |     match Some(0) {
    |           ------- this expression has type `Option<{integer}>`
@@ -28,6 +9,6 @@ LL |         Ok(0) => {}
    = note: expected enum `Option<{integer}>`
               found enum `Result<_, _>`
 
-error: aborting due to 3 previous errors
+error: aborting due to 1 previous error
 
 For more information about this error, try `rustc --explain E0308`.