From: bors <bors@rust-lang.org>
Date: Sun, 1 Nov 2020 14:37:50 +0000 (+0000)
Subject: Auto merge of #78553 - Nadrieril:fix-78549, r=varkor
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=1899c489d4c30b2640d30b77ac04f0a548834d81;hp=-c;p=rust.git

Auto merge of #78553 - Nadrieril:fix-78549, r=varkor

Fix #78549

Before #78430, this worked because `specialize_constructor` didn't actually care too much which constructor was passed to it unless needed. That PR however handles `&str` as a special case, and I did not anticipate patterns for the `&str` type other than string literals.
I am not very confident there are not other similar oversights left, but hopefully only `&str` was different enough to break my assumptions.

Fixes https://github.com/rust-lang/rust/issues/78549
---

1899c489d4c30b2640d30b77ac04f0a548834d81
diff --combined compiler/rustc_mir_build/src/thir/pattern/_match.rs
index 2da5ae574bb,4bbbf416bec..e0de1351ac3
--- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs
@@@ -327,9 -327,23 +327,23 @@@ struct LiteralExpander
  impl<'tcx> PatternFolder<'tcx> for LiteralExpander {
      fn fold_pattern(&mut self, pat: &Pat<'tcx>) -> Pat<'tcx> {
          debug!("fold_pattern {:?} {:?} {:?}", pat, pat.ty.kind(), pat.kind);
-         match (pat.ty.kind(), &*pat.kind) {
-             (_, &PatKind::Binding { subpattern: Some(ref s), .. }) => s.fold_with(self),
-             (_, &PatKind::AscribeUserType { subpattern: ref s, .. }) => s.fold_with(self),
+         match (pat.ty.kind(), pat.kind.as_ref()) {
+             (_, PatKind::Binding { subpattern: Some(s), .. }) => s.fold_with(self),
+             (_, PatKind::AscribeUserType { subpattern: s, .. }) => s.fold_with(self),
+             (ty::Ref(_, t, _), PatKind::Constant { .. }) if t.is_str() => {
+                 // Treat string literal patterns as deref patterns to a `str` constant, i.e.
+                 // `&CONST`. This expands them like other const patterns. This could have been done
+                 // in `const_to_pat`, but that causes issues with the rest of the matching code.
+                 let mut new_pat = pat.super_fold_with(self);
+                 // Make a fake const pattern of type `str` (instead of `&str`). That the carried
+                 // constant value still knows it is of type `&str`.
+                 new_pat.ty = t;
+                 Pat {
+                     kind: Box::new(PatKind::Deref { subpattern: new_pat }),
+                     span: pat.span,
+                     ty: pat.ty,
+                 }
+             }
              _ => pat.super_fold_with(self),
          }
      }
@@@ -337,7 -351,10 +351,7 @@@
  
  impl<'tcx> Pat<'tcx> {
      pub(super) fn is_wildcard(&self) -> bool {
 -        match *self.kind {
 -            PatKind::Binding { subpattern: None, .. } | PatKind::Wild => true,
 -            _ => false,
 -        }
 +        matches!(*self.kind, PatKind::Binding { subpattern: None, .. } | PatKind::Wild)
      }
  }
  
@@@ -782,11 -799,9 +796,9 @@@ enum Constructor<'tcx> 
      /// boxes for the purposes of exhaustiveness: we must not inspect them, and they
      /// don't count towards making a match exhaustive.
      Opaque,
-     /// Fake extra constructor for enums that aren't allowed to be matched exhaustively.
+     /// Fake extra constructor for enums that aren't allowed to be matched exhaustively. Also used
+     /// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
      NonExhaustive,
-     /// Fake constructor for those types for which we can't list constructors explicitly, like
-     /// `f64` and `&str`.
-     Unlistable,
      /// Wildcard pattern.
      Wildcard,
  }
@@@ -880,6 -895,7 +892,7 @@@ impl<'tcx> Constructor<'tcx> 
      /// For the simple cases, this is simply checking for equality. For the "grouped" constructors,
      /// this checks for inclusion.
      fn is_covered_by<'p>(&self, pcx: PatCtxt<'_, 'p, 'tcx>, other: &Self) -> bool {
+         // This must be kept in sync with `is_covered_by_any`.
          match (self, other) {
              // Wildcards cover anything
              (_, Wildcard) => true,
@@@ -922,18 -938,19 +935,19 @@@
              (Opaque, _) | (_, Opaque) => false,
              // Only a wildcard pattern can match the special extra constructor.
              (NonExhaustive, _) => false,
-             // If we encounter a `Single` here, this means there was only one constructor for this
-             // type after all.
-             (Unlistable, Single) => true,
-             // Otherwise, only a wildcard pattern can match the special extra constructor.
-             (Unlistable, _) => false,
  
-             _ => bug!("trying to compare incompatible constructors {:?} and {:?}", self, other),
+             _ => span_bug!(
+                 pcx.span,
+                 "trying to compare incompatible constructors {:?} and {:?}",
+                 self,
+                 other
+             ),
          }
      }
  
      /// Faster version of `is_covered_by` when applied to many constructors. `used_ctors` is
-     /// assumed to be built from `matrix.head_ctors()`, and `self` is assumed to have been split.
+     /// assumed to be built from `matrix.head_ctors()` with wildcards filtered out, and `self` is
+     /// assumed to have been split from a wildcard.
      fn is_covered_by_any<'p>(
          &self,
          pcx: PatCtxt<'_, 'p, 'tcx>,
@@@ -943,8 -960,9 +957,9 @@@
              return false;
          }
  
+         // This must be kept in sync with `is_covered_by`.
          match self {
-             // `used_ctors` cannot contain anything else than `Single`s.
+             // If `self` is `Single`, `used_ctors` cannot contain anything else than `Single`s.
              Single => !used_ctors.is_empty(),
              Variant(_) => used_ctors.iter().any(|c| c == self),
              IntRange(range) => used_ctors
@@@ -957,8 -975,6 +972,6 @@@
                  .any(|other| slice.is_covered_by(other)),
              // This constructor is never covered by anything else
              NonExhaustive => false,
-             // This constructor is only covered by `Single`s
-             Unlistable => used_ctors.iter().any(|c| *c == Single),
              Str(..) | FloatRange(..) | Opaque | Wildcard => {
                  bug!("found unexpected ctor in all_ctors: {:?}", self)
              }
@@@ -1006,6 -1022,10 +1019,10 @@@
                          PatKind::Leaf { subpatterns }
                      }
                  }
+                 // Note: given the expansion of `&str` patterns done in `expand_pattern`, we should
+                 // be careful to reconstruct the correct constant pattern here. However a string
+                 // literal pattern will never be reported as a non-exhaustiveness witness, so we
+                 // can ignore this issue.
                  ty::Ref(..) => PatKind::Deref { subpattern: subpatterns.next().unwrap() },
                  ty::Slice(_) | ty::Array(..) => bug!("bad slice pattern {:?} {:?}", self, pcx.ty),
                  _ => PatKind::Wild,
@@@ -1038,7 -1058,7 +1055,7 @@@
              &Str(value) => PatKind::Constant { value },
              &FloatRange(lo, hi, end) => PatKind::Range(PatRange { lo, hi, end }),
              IntRange(range) => return range.to_pat(pcx.cx.tcx),
-             NonExhaustive | Unlistable => PatKind::Wild,
+             NonExhaustive => PatKind::Wild,
              Opaque => bug!("we should not try to apply an opaque constructor"),
              Wildcard => bug!(
                  "trying to apply a wildcard constructor; this should have been done in `apply_constructors`"
@@@ -1187,8 -1207,9 +1204,9 @@@ impl<'p, 'tcx> Fields<'p, 'tcx> 
                  }
                  _ => bug!("bad slice pattern {:?} {:?}", constructor, ty),
              },
-             Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Unlistable
-             | Wildcard => Fields::empty(),
+             Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Wildcard => {
+                 Fields::empty()
+             }
          };
          debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret);
          ret
@@@ -1300,9 -1321,13 +1318,13 @@@
      ///     [Some(0), ..] => {}
      /// }
      /// ```
+     /// This is guaranteed to preserve the number of patterns in `self`.
      fn replace_with_pattern_arguments(&self, pat: &'p Pat<'tcx>) -> Self {
          match pat.kind.as_ref() {
-             PatKind::Deref { subpattern } => Self::from_single_pattern(subpattern),
+             PatKind::Deref { subpattern } => {
+                 assert_eq!(self.len(), 1);
+                 Fields::from_single_pattern(subpattern)
+             }
              PatKind::Leaf { subpatterns } | PatKind::Variant { subpatterns, .. } => {
                  self.replace_with_fieldpats(subpatterns)
              }
@@@ -1355,7 -1380,10 +1377,7 @@@ impl<'tcx> Usefulness<'tcx> 
      }
  
      fn is_useful(&self) -> bool {
 -        match *self {
 -            NotUseful => false,
 -            _ => true,
 -        }
 +        !matches!(*self, NotUseful)
      }
  
      fn apply_constructor<'p>(
@@@ -1590,10 -1618,9 +1612,9 @@@ fn all_constructors<'p, 'tcx>(pcx: PatC
              vec![make_range(0, max)]
          }
          _ if cx.is_uninhabited(pcx.ty) => vec![],
-         ty::Adt(..) | ty::Tuple(..) => vec![Single],
-         ty::Ref(_, t, _) if !t.is_str() => vec![Single],
-         // This type is one for which we don't know how to list constructors, like `&str` or `f64`.
-         _ => vec![Unlistable],
+         ty::Adt(..) | ty::Tuple(..) | ty::Ref(..) => vec![Single],
+         // This type is one for which we cannot list constructors, like `str` or `f64`.
+         _ => vec![NonExhaustive],
      }
  }
  
@@@ -1617,7 -1644,10 +1638,7 @@@ struct IntRange<'tcx> 
  impl<'tcx> IntRange<'tcx> {
      #[inline]
      fn is_integral(ty: Ty<'_>) -> bool {
 -        match ty.kind() {
 -            ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool => true,
 -            _ => false,
 -        }
 +        matches!(ty.kind(), ty::Char | ty::Int(_) | ty::Uint(_) | ty::Bool)
      }
  
      fn is_singleton(&self) -> bool {
@@@ -2152,20 -2182,23 +2173,23 @@@ fn pat_constructor<'p, 'tcx>
      cx: &MatchCheckCtxt<'p, 'tcx>,
      pat: &'p Pat<'tcx>,
  ) -> Constructor<'tcx> {
-     match *pat.kind {
+     match pat.kind.as_ref() {
          PatKind::AscribeUserType { .. } => bug!(), // Handled by `expand_pattern`
          PatKind::Binding { .. } | PatKind::Wild => Wildcard,
          PatKind::Leaf { .. } | PatKind::Deref { .. } => Single,
-         PatKind::Variant { adt_def, variant_index, .. } => {
+         &PatKind::Variant { adt_def, variant_index, .. } => {
              Variant(adt_def.variants[variant_index].def_id)
          }
          PatKind::Constant { value } => {
              if let Some(int_range) = IntRange::from_const(cx.tcx, cx.param_env, value, pat.span) {
                  IntRange(int_range)
              } else {
-                 match value.ty.kind() {
+                 match pat.ty.kind() {
                      ty::Float(_) => FloatRange(value, value, RangeEnd::Included),
-                     ty::Ref(_, t, _) if t.is_str() => Str(value),
+                     // In `expand_pattern`, we convert string literals to `&CONST` patterns with
+                     // `CONST` a pattern of type `str`. In truth this contains a constant of type
+                     // `&str`.
+                     ty::Str => Str(value),
                      // All constants that can be structurally matched have already been expanded
                      // into the corresponding `Pat`s by `const_to_pat`. Constants that remain are
                      // opaque.
@@@ -2173,7 -2206,7 +2197,7 @@@
                  }
              }
          }
-         PatKind::Range(PatRange { lo, hi, end }) => {
+         &PatKind::Range(PatRange { lo, hi, end }) => {
              let ty = lo.ty;
              if let Some(int_range) = IntRange::from_range(
                  cx.tcx,
@@@ -2188,8 -2221,7 +2212,7 @@@
                  FloatRange(lo, hi, end)
              }
          }
-         PatKind::Array { ref prefix, ref slice, ref suffix }
-         | PatKind::Slice { ref prefix, ref slice, ref suffix } => {
+         PatKind::Array { prefix, slice, suffix } | PatKind::Slice { prefix, slice, suffix } => {
              let array_len = match pat.ty.kind() {
                  ty::Array(_, length) => Some(length.eval_usize(cx.tcx, cx.param_env)),
                  ty::Slice(_) => None,