Auto merge of #151155 - Zalathar:str, r=<try>

THIR patterns: Always use type `str` for string-constant-value nodes

Author: bors

compiler/rustc_mir_build/src/builder/matches/match_pair.rs

@@ -2,7 +2,6 @@ use std::sync::Arc;
 
 use rustc_abi::FieldIdx;
 use rustc_middle::mir::*;
-use rustc_middle::span_bug;
 use rustc_middle::thir::*;
 use rustc_middle::ty::{self, Ty, TypeVisitableExt};
 
@@ -174,10 +173,7 @@ impl<'tcx> MatchPairTree<'tcx> {
             }
 
             PatKind::Constant { value } => {
-                // CAUTION: The type of the pattern node (`pattern.ty`) is
-                // _often_ the same as the type of the const value (`value.ty`),
-                // but there are some cases where those types differ
-                // (e.g. when `deref!(..)` patterns interact with `String`).
+                assert_eq!(pattern.ty, value.ty);
 
                 // Classify the constant-pattern into further kinds, to
                 // reduce the number of ad-hoc type tests needed later on.
@@ -189,16 +185,6 @@ impl<'tcx> MatchPairTree<'tcx> {
                 } else if pat_ty.is_floating_point() {
                     PatConstKind::Float
                 } else if pat_ty.is_str() {
-                    // Deref-patterns can cause string-literal patterns to have
-                    // type `str` instead of the usual `&str`.
-                    if !cx.tcx.features().deref_patterns() {
-                        span_bug!(
-                            pattern.span,
-                            "const pattern has type `str` but deref_patterns is not enabled"
-                        );
-                    }
-                    PatConstKind::String
-                } else if pat_ty.is_imm_ref_str() {
                     PatConstKind::String
                 } else {
                     // FIXME(Zalathar): This still covers several different

compiler/rustc_mir_build/src/builder/matches/mod.rs

@@ -1339,19 +1339,13 @@ enum TestKind<'tcx> {
 
     /// Tests the place against a string constant using string equality.
     StringEq {
-        /// Constant `&str` value to test against.
+        /// Constant string value to test against.
+        /// Note that this value has type `str` (not `&str`).
         value: ty::Value<'tcx>,
-        /// Type of the corresponding pattern node. Usually `&str`, but could
-        /// be `str` for patterns like `deref!("..."): String`.
-        pat_ty: Ty<'tcx>,
     },
 
     /// Tests the place against a constant using scalar equality.
-    ScalarEq {
-        value: ty::Value<'tcx>,
-        /// Type of the corresponding pattern node.
-        pat_ty: Ty<'tcx>,
-    },
+    ScalarEq { value: ty::Value<'tcx> },
 
     /// Test whether the value falls within an inclusive or exclusive range.
     Range(Arc<PatRange<'tcx>>),

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

@@ -9,10 +9,10 @@ use std::sync::Arc;
 
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir::{LangItem, RangeEnd};
+use rustc_middle::bug;
 use rustc_middle::mir::*;
 use rustc_middle::ty::util::IntTypeExt;
 use rustc_middle::ty::{self, GenericArg, Ty, TyCtxt};
-use rustc_middle::{bug, span_bug};
 use rustc_span::def_id::DefId;
 use rustc_span::source_map::Spanned;
 use rustc_span::{DUMMY_SP, Span, Symbol, sym};
@@ -39,10 +39,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 TestKind::SwitchInt
             }
             TestableCase::Constant { value, kind: PatConstKind::String } => {
-                TestKind::StringEq { value, pat_ty: match_pair.pattern_ty }
+                TestKind::StringEq { value }
             }
             TestableCase::Constant { value, kind: PatConstKind::Float | PatConstKind::Other } => {
-                TestKind::ScalarEq { value, pat_ty: match_pair.pattern_ty }
+                TestKind::ScalarEq { value }
             }
 
             TestableCase::Range(ref range) => {
@@ -141,47 +141,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 self.cfg.terminate(block, self.source_info(match_start_span), terminator);
             }
 
-            TestKind::StringEq { value, pat_ty } => {
+            TestKind::StringEq { value } => {
                 let tcx = self.tcx;
                 let success_block = target_block(TestBranch::Success);
                 let fail_block = target_block(TestBranch::Failure);
 
-                let expected_value_ty = value.ty;
-                let expected_value_operand =
-                    self.literal_operand(test.span, Const::from_ty_value(tcx, value));
+                let ref_str_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, tcx.types.str_);
+                assert!(ref_str_ty.is_imm_ref_str(), "{ref_str_ty:?}");
 
-                let mut actual_value_ty = pat_ty;
-                let mut actual_value_place = place;
-
-                match pat_ty.kind() {
-                    ty::Str => {
-                        // String literal patterns may have type `str` if `deref_patterns` is
-                        // enabled, in order to allow `deref!("..."): String`. In this case, `value`
-                        // is of type `&str`, so we compare it to `&place`.
-                        if !tcx.features().deref_patterns() {
-                            span_bug!(
-                                test.span,
-                                "matching on `str` went through without enabling deref_patterns"
-                            );
-                        }
-                        let re_erased = tcx.lifetimes.re_erased;
-                        let ref_str_ty = Ty::new_imm_ref(tcx, re_erased, tcx.types.str_);
-                        let ref_place = self.temp(ref_str_ty, test.span);
-                        // `let ref_place: &str = &place;`
-                        self.cfg.push_assign(
-                            block,
-                            self.source_info(test.span),
-                            ref_place,
-                            Rvalue::Ref(re_erased, BorrowKind::Shared, place),
-                        );
-                        actual_value_place = ref_place;
-                        actual_value_ty = ref_str_ty;
-                    }
-                    _ => {}
-                }
+                // The string constant we're testing against has type `str`, but
+                // calling `<str as PartialEq>::eq` requires `&str` operands.
+                //
+                // Because `str` and `&str` have the same valtree representation,
+                // we can "cast" to the desired type by just replacing the type.
+                assert!(value.ty.is_str(), "unexpected value type for StringEq test: {value:?}");
+                let expected_value = ty::Value { ty: ref_str_ty, valtree: value.valtree };
+                let expected_value_operand =
+                    self.literal_operand(test.span, Const::from_ty_value(tcx, expected_value));
 
-                assert_eq!(expected_value_ty, actual_value_ty);
-                assert!(actual_value_ty.is_imm_ref_str());
+                // Similarly, the scrutinized place has type `str`, but we need `&str`.
+                // Get a reference by doing `let actual_value_ref_place: &str = &place`.
+                let actual_value_ref_place = self.temp(ref_str_ty, test.span);
+                self.cfg.push_assign(
+                    block,
+                    self.source_info(test.span),
+                    actual_value_ref_place,
+                    Rvalue::Ref(tcx.lifetimes.re_erased, BorrowKind::Shared, place),
+                );
 
                 // Compare two strings using `<str as std::cmp::PartialEq>::eq`.
                 // (Interestingly this means that exhaustiveness analysis relies, for soundness,
@@ -192,11 +178,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     fail_block,
                     source_info,
                     expected_value_operand,
-                    Operand::Copy(actual_value_place),
+                    Operand::Copy(actual_value_ref_place),
                 );
             }
 
-            TestKind::ScalarEq { value, pat_ty } => {
+            TestKind::ScalarEq { value } => {
                 let tcx = self.tcx;
                 let success_block = target_block(TestBranch::Success);
                 let fail_block = target_block(TestBranch::Failure);
@@ -205,12 +191,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let mut expected_value_operand =
                     self.literal_operand(test.span, Const::from_ty_value(tcx, value));
 
-                let mut actual_value_ty = pat_ty;
                 let mut actual_value_place = place;
 
-                match pat_ty.kind() {
+                match value.ty.kind() {
                     &ty::Pat(base, _) => {
-                        assert_eq!(pat_ty, value.ty);
                         assert!(base.is_trivially_pure_clone_copy());
 
                         let transmuted_place = self.temp(base, test.span);
@@ -234,15 +218,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         );
 
                         actual_value_place = transmuted_place;
-                        actual_value_ty = base;
                         expected_value_operand = Operand::Copy(transmuted_expect);
                         expected_value_ty = base;
                     }
                     _ => {}
                 }
 
-                assert_eq!(expected_value_ty, actual_value_ty);
-                assert!(actual_value_ty.is_scalar());
+                assert!(expected_value_ty.is_scalar());
 
                 self.compare(
                     block,

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

@@ -289,32 +289,29 @@ impl<'tcx> ConstToPat<'tcx> {
                 suffix: Box::new([]),
             },
             ty::Str => {
-                // String literal patterns may have type `str` if `deref_patterns` is enabled, in
-                // order to allow `deref!("..."): String`. Since we need a `&str` for the comparison
-                // when lowering to MIR in `Builder::perform_test`, treat the constant as a `&str`.
-                // This works because `str` and `&str` have the same valtree representation.
-                let ref_str_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, ty);
-                PatKind::Constant { value: ty::Value { ty: ref_str_ty, valtree: cv } }
+                // Constant/literal patterns of type `&str` are lowered to a
+                // `PatKind::Deref` wrapping a `PatKind::Constant` of type `str`.
+                // This pattern node is the `str` constant part.
+                //
+                // Under `feature(deref_patterns)`, string literal patterns can also
+                // have type `str` directly, without the `&`, in order to allow things
+                // like `deref!("...")` to work when the scrutinee is `String`.
+                PatKind::Constant { value: ty::Value { ty, valtree: cv } }
             }
-            ty::Ref(_, pointee_ty, ..) => match *pointee_ty.kind() {
-                // `&str` is represented as a valtree, let's keep using this
-                // optimization for now.
-                ty::Str => PatKind::Constant { value: ty::Value { ty, valtree: 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.
-                _ => {
-                    if !pointee_ty.is_sized(tcx, self.typing_env) && !pointee_ty.is_slice() {
-                        return self.mk_err(
-                            tcx.dcx().create_err(UnsizedPattern { span, non_sm_ty: *pointee_ty }),
-                            ty,
-                        );
-                    } else {
-                        // References have the same valtree representation as their pointee.
-                        PatKind::Deref { subpattern: self.valtree_to_pat(cv, *pointee_ty) }
-                    }
+            ty::Ref(_, pointee_ty, ..) => {
+                if pointee_ty.is_str()
+                    || pointee_ty.is_slice()
+                    || pointee_ty.is_sized(tcx, self.typing_env)
+                {
+                    // References have the same valtree representation as their pointee.
+                    PatKind::Deref { subpattern: self.valtree_to_pat(cv, *pointee_ty) }
+                } else {
+                    return self.mk_err(
+                        tcx.dcx().create_err(UnsizedPattern { span, non_sm_ty: *pointee_ty }),
+                        ty,
+                    );
                 }
-            },
+            }
             ty::Float(flt) => {
                 let v = cv.to_leaf();
                 let is_nan = match flt {

compiler/rustc_pattern_analysis/src/rustc.rs

@@ -583,19 +583,13 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
                         fields = vec![];
                         arity = 0;
                     }
-                    ty::Ref(_, t, _) if t.is_str() => {
-                        // We want a `&str` constant to behave like a `Deref` pattern, to be compatible
-                        // with other `Deref` patterns. This could have been done in `const_to_pat`,
-                        // but that causes issues with the rest of the matching code.
-                        // So here, the constructor for a `"foo"` pattern is `&` (represented by
-                        // `Ref`), and has one field. That field has constructor `Str(value)` and no
-                        // subfields.
-                        // Note: `t` is `str`, not `&str`.
-                        let ty = self.reveal_opaque_ty(*t);
-                        let subpattern = DeconstructedPat::new(Str(*value), Vec::new(), 0, ty, pat);
-                        ctor = Ref;
-                        fields = vec![subpattern.at_index(0)];
-                        arity = 1;
+                    ty::Str => {
+                        // For constant/literal patterns of type `&str`, the THIR
+                        // pattern is a `PatKind::Deref` of type `&str` wrapping a
+                        // `PatKind::Const` of type `str`.
+                        ctor = Str(*value);
+                        fields = vec![];
+                        arity = 0;
                     }
                     // All constants that can be structurally matched have already been expanded
                     // into the corresponding `Pat`s by `const_to_pat`. Constants that remain are

tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir

@@ -7,11 +7,14 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     let mut _3: (&str, bool);
     let mut _4: &str;
     let mut _5: bool;
-    let mut _6: &&str;
-    let mut _7: &bool;
-    let mut _8: bool;
-    let mut _9: bool;
+    let mut _6: &str;
+    let mut _7: &&str;
+    let mut _8: &bool;
+    let mut _9: &str;
     let mut _10: bool;
+    let mut _11: &str;
+    let mut _12: bool;
+    let mut _13: bool;
 
     bb0: {
         StorageLive(_3);
@@ -23,7 +26,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
         StorageDead(_5);
         StorageDead(_4);
         PlaceMention(_3);
-        _9 = <str as PartialEq>::eq(copy (_3.0: &str), const "a") -> [return: bb9, unwind: bb19];
+        _11 = &(*(_3.0: &str));
+        _12 = <str as PartialEq>::eq(copy _11, const "a") -> [return: bb9, unwind: bb19];
     }
 
     bb1: {
@@ -43,7 +47,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     }
 
     bb5: {
-        _8 = <str as PartialEq>::eq(copy (_3.0: &str), const "b") -> [return: bb8, unwind: bb19];
+        _9 = &(*(_3.0: &str));
+        _10 = <str as PartialEq>::eq(copy _9, const "b") -> [return: bb8, unwind: bb19];
     }
 
     bb6: {
@@ -55,11 +60,11 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     }
 
     bb8: {
-        switchInt(move _8) -> [0: bb1, otherwise: bb6];
+        switchInt(move _10) -> [0: bb1, otherwise: bb6];
     }
 
     bb9: {
-        switchInt(move _9) -> [0: bb5, otherwise: bb2];
+        switchInt(move _12) -> [0: bb5, otherwise: bb2];
     }
 
     bb10: {
@@ -87,23 +92,25 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     }
 
     bb15: {
-        _6 = &fake shallow (_3.0: &str);
-        _7 = &fake shallow (_3.1: bool);
-        StorageLive(_10);
-        _10 = const true;
-        switchInt(move _10) -> [0: bb17, otherwise: bb16];
+        _6 = &fake shallow (*(_3.0: &str));
+        _7 = &fake shallow (_3.0: &str);
+        _8 = &fake shallow (_3.1: bool);
+        StorageLive(_13);
+        _13 = const true;
+        switchInt(move _13) -> [0: bb17, otherwise: bb16];
     }
 
     bb16: {
-        StorageDead(_10);
+        StorageDead(_13);
         FakeRead(ForMatchGuard, _6);
         FakeRead(ForMatchGuard, _7);
+        FakeRead(ForMatchGuard, _8);
         _0 = const 1_u32;
         goto -> bb18;
     }
 
     bb17: {
-        StorageDead(_10);
+        StorageDead(_13);
         falseEdge -> [real: bb3, imaginary: bb5];
     }
 

tests/ui/thir-print/str-patterns.rs

@@ -0,0 +1,17 @@
+#![crate_type = "rlib"]
+//@ edition: 2024
+//@ compile-flags: -Zunpretty=thir-flat
+//@ check-pass
+
+// Snapshot test capturing the THIR pattern structure produced by
+// string-literal and string-constant patterns.
+
+pub fn hello_world(x: &str) {
+    match x {
+        "hello" => {}
+        CONSTANT => {}
+        _ => {}
+    }
+}
+
+const CONSTANT: &str = "constant";