Rollup merge of #109838 - clubby789:non-exhaustive-span, r=Nilstrieb

Fix `non_exhaustive_omitted_patterns` lint span

Fixes #109837

`DUMMY_SP` was being passed as the span in many cases where we have a span available to use. This meant that the location of the violating pattern wasn't shown, or the list of un-covered variants

r? `@Nilstrieb`
This commit is contained in:
Michael Goulet 2023-04-04 09:27:44 -07:00 committed by GitHub
commit 4d32de6fcb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 46 additions and 21 deletions

View File

@ -1154,8 +1154,9 @@ pub(super) fn from_iter(
fn wildcards_from_tys( fn wildcards_from_tys(
cx: &MatchCheckCtxt<'p, 'tcx>, cx: &MatchCheckCtxt<'p, 'tcx>,
tys: impl IntoIterator<Item = Ty<'tcx>>, tys: impl IntoIterator<Item = Ty<'tcx>>,
span: Span,
) -> Self { ) -> Self {
Fields::from_iter(cx, tys.into_iter().map(DeconstructedPat::wildcard)) Fields::from_iter(cx, tys.into_iter().map(|ty| DeconstructedPat::wildcard(ty, span)))
} }
// In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide // In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide
@ -1191,18 +1192,18 @@ fn list_variant_nonhidden_fields<'a>(
pub(super) fn wildcards(pcx: &PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'tcx>) -> Self { pub(super) fn wildcards(pcx: &PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'tcx>) -> Self {
let ret = match constructor { let ret = match constructor {
Single | Variant(_) => match pcx.ty.kind() { Single | Variant(_) => match pcx.ty.kind() {
ty::Tuple(fs) => Fields::wildcards_from_tys(pcx.cx, fs.iter()), ty::Tuple(fs) => Fields::wildcards_from_tys(pcx.cx, fs.iter(), pcx.span),
ty::Ref(_, rty, _) => Fields::wildcards_from_tys(pcx.cx, once(*rty)), ty::Ref(_, rty, _) => Fields::wildcards_from_tys(pcx.cx, once(*rty), pcx.span),
ty::Adt(adt, substs) => { ty::Adt(adt, substs) => {
if adt.is_box() { if adt.is_box() {
// The only legal patterns of type `Box` (outside `std`) are `_` and box // The only legal patterns of type `Box` (outside `std`) are `_` and box
// patterns. If we're here we can assume this is a box pattern. // patterns. If we're here we can assume this is a box pattern.
Fields::wildcards_from_tys(pcx.cx, once(substs.type_at(0))) Fields::wildcards_from_tys(pcx.cx, once(substs.type_at(0)), pcx.span)
} else { } else {
let variant = &adt.variant(constructor.variant_index_for_adt(*adt)); let variant = &adt.variant(constructor.variant_index_for_adt(*adt));
let tys = Fields::list_variant_nonhidden_fields(pcx.cx, pcx.ty, variant) let tys = Fields::list_variant_nonhidden_fields(pcx.cx, pcx.ty, variant)
.map(|(_, ty)| ty); .map(|(_, ty)| ty);
Fields::wildcards_from_tys(pcx.cx, tys) Fields::wildcards_from_tys(pcx.cx, tys, pcx.span)
} }
} }
_ => bug!("Unexpected type for `Single` constructor: {:?}", pcx), _ => bug!("Unexpected type for `Single` constructor: {:?}", pcx),
@ -1210,7 +1211,7 @@ pub(super) fn wildcards(pcx: &PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'
Slice(slice) => match *pcx.ty.kind() { Slice(slice) => match *pcx.ty.kind() {
ty::Slice(ty) | ty::Array(ty, _) => { ty::Slice(ty) | ty::Array(ty, _) => {
let arity = slice.arity(); let arity = slice.arity();
Fields::wildcards_from_tys(pcx.cx, (0..arity).map(|_| ty)) Fields::wildcards_from_tys(pcx.cx, (0..arity).map(|_| ty), pcx.span)
} }
_ => bug!("bad slice pattern {:?} {:?}", constructor, pcx), _ => bug!("bad slice pattern {:?} {:?}", constructor, pcx),
}, },
@ -1251,8 +1252,8 @@ pub(crate) struct DeconstructedPat<'p, 'tcx> {
} }
impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> { impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
pub(super) fn wildcard(ty: Ty<'tcx>) -> Self { pub(super) fn wildcard(ty: Ty<'tcx>, span: Span) -> Self {
Self::new(Wildcard, Fields::empty(), ty, DUMMY_SP) Self::new(Wildcard, Fields::empty(), ty, span)
} }
pub(super) fn new( pub(super) fn new(
@ -1269,7 +1270,7 @@ pub(super) fn new(
/// `Some(_)`. /// `Some(_)`.
pub(super) fn wild_from_ctor(pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: Constructor<'tcx>) -> Self { pub(super) fn wild_from_ctor(pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: Constructor<'tcx>) -> Self {
let fields = Fields::wildcards(pcx, &ctor); let fields = Fields::wildcards(pcx, &ctor);
DeconstructedPat::new(ctor, fields, pcx.ty, DUMMY_SP) DeconstructedPat::new(ctor, fields, pcx.ty, pcx.span)
} }
/// Clone this value. This method emphasizes that cloning loses reachability information and /// Clone this value. This method emphasizes that cloning loses reachability information and
@ -1298,7 +1299,7 @@ pub(crate) fn from_pat(cx: &MatchCheckCtxt<'p, 'tcx>, pat: &Pat<'tcx>) -> Self {
ty::Tuple(fs) => { ty::Tuple(fs) => {
ctor = Single; ctor = Single;
let mut wilds: SmallVec<[_; 2]> = let mut wilds: SmallVec<[_; 2]> =
fs.iter().map(DeconstructedPat::wildcard).collect(); fs.iter().map(|ty| DeconstructedPat::wildcard(ty, pat.span)).collect();
for pat in subpatterns { for pat in subpatterns {
wilds[pat.field.index()] = mkpat(&pat.pattern); wilds[pat.field.index()] = mkpat(&pat.pattern);
} }
@ -1317,11 +1318,11 @@ pub(crate) fn from_pat(cx: &MatchCheckCtxt<'p, 'tcx>, pat: &Pat<'tcx>) -> Self {
// normally or through box-patterns. We'll have to figure out a proper // normally or through box-patterns. We'll have to figure out a proper
// solution when we introduce generalized deref patterns. Also need to // solution when we introduce generalized deref patterns. Also need to
// prevent mixing of those two options. // prevent mixing of those two options.
let pat = subpatterns.into_iter().find(|pat| pat.field.index() == 0); let pattern = subpatterns.into_iter().find(|pat| pat.field.index() == 0);
let pat = if let Some(pat) = pat { let pat = if let Some(pat) = pattern {
mkpat(&pat.pattern) mkpat(&pat.pattern)
} else { } else {
DeconstructedPat::wildcard(substs.type_at(0)) DeconstructedPat::wildcard(substs.type_at(0), pat.span)
}; };
ctor = Single; ctor = Single;
fields = Fields::singleton(cx, pat); fields = Fields::singleton(cx, pat);
@ -1343,7 +1344,7 @@ pub(crate) fn from_pat(cx: &MatchCheckCtxt<'p, 'tcx>, pat: &Pat<'tcx>) -> Self {
ty ty
}); });
let mut wilds: SmallVec<[_; 2]> = let mut wilds: SmallVec<[_; 2]> =
tys.map(DeconstructedPat::wildcard).collect(); tys.map(|ty| DeconstructedPat::wildcard(ty, pat.span)).collect();
for pat in subpatterns { for pat in subpatterns {
if let Some(i) = field_id_to_id[pat.field.index()] { if let Some(i) = field_id_to_id[pat.field.index()] {
wilds[i] = mkpat(&pat.pattern); wilds[i] = mkpat(&pat.pattern);
@ -1566,8 +1567,10 @@ pub(super) fn specialize<'a>(
}; };
let prefix = &self.fields.fields[..prefix]; let prefix = &self.fields.fields[..prefix];
let suffix = &self.fields.fields[self_slice.arity() - suffix..]; let suffix = &self.fields.fields[self_slice.arity() - suffix..];
let wildcard: &_ = let wildcard: &_ = pcx
pcx.cx.pattern_arena.alloc(DeconstructedPat::wildcard(inner_ty)); .cx
.pattern_arena
.alloc(DeconstructedPat::wildcard(inner_ty, pcx.span));
let extra_wildcards = other_slice.arity() - self_slice.arity(); let extra_wildcards = other_slice.arity() - self_slice.arity();
let extra_wildcards = (0..extra_wildcards).map(|_| wildcard); let extra_wildcards = (0..extra_wildcards).map(|_| wildcard);
prefix.iter().chain(extra_wildcards).chain(suffix).collect() prefix.iter().chain(extra_wildcards).chain(suffix).collect()

View File

@ -604,7 +604,7 @@ fn apply_constructor(
let new_patterns = if pcx.is_non_exhaustive { let new_patterns = if pcx.is_non_exhaustive {
// Here we don't want the user to try to list all variants, we want them to add // Here we don't want the user to try to list all variants, we want them to add
// a wildcard, so we only suggest that. // a wildcard, so we only suggest that.
vec![DeconstructedPat::wildcard(pcx.ty)] vec![DeconstructedPat::wildcard(pcx.ty, pcx.span)]
} else { } else {
let mut split_wildcard = SplitWildcard::new(pcx); let mut split_wildcard = SplitWildcard::new(pcx);
split_wildcard.split(pcx, matrix.heads().map(DeconstructedPat::ctor)); split_wildcard.split(pcx, matrix.heads().map(DeconstructedPat::ctor));
@ -631,7 +631,7 @@ fn apply_constructor(
.collect(); .collect();
if hide_variant_show_wild { if hide_variant_show_wild {
new.push(DeconstructedPat::wildcard(pcx.ty)); new.push(DeconstructedPat::wildcard(pcx.ty, pcx.span));
} }
new new
@ -734,7 +734,7 @@ fn apply_constructor(mut self, pcx: &PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'
let arity = ctor.arity(pcx); let arity = ctor.arity(pcx);
let pats = self.0.drain((len - arity)..).rev(); let pats = self.0.drain((len - arity)..).rev();
let fields = Fields::from_iter(pcx.cx, pats); let fields = Fields::from_iter(pcx.cx, pats);
DeconstructedPat::new(ctor.clone(), fields, pcx.ty, DUMMY_SP) DeconstructedPat::new(ctor.clone(), fields, pcx.ty, pcx.span)
}; };
self.0.push(pat); self.0.push(pat);
@ -977,7 +977,7 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
}) })
.collect(); .collect();
let wild_pattern = cx.pattern_arena.alloc(DeconstructedPat::wildcard(scrut_ty)); let wild_pattern = cx.pattern_arena.alloc(DeconstructedPat::wildcard(scrut_ty, DUMMY_SP));
let v = PatStack::from_pattern(wild_pattern); let v = PatStack::from_pattern(wild_pattern);
let usefulness = is_useful(cx, &matrix, &v, FakeExtraWildcard, scrut_hir_id, false, true); let usefulness = is_useful(cx, &matrix, &v, FakeExtraWildcard, scrut_hir_id, false, true);
let non_exhaustiveness_witnesses = match usefulness { let non_exhaustiveness_witnesses = match usefulness {

View File

@ -194,6 +194,14 @@ fn main() {
let local_refutable @ NonExhaustiveEnum::Unit = NonExhaustiveEnum::Unit; let local_refutable @ NonExhaustiveEnum::Unit = NonExhaustiveEnum::Unit;
//~^ refutable pattern in local binding //~^ refutable pattern in local binding
// Check that matching on a reference results in a correctly spanned diagnostic
#[deny(non_exhaustive_omitted_patterns)]
match &non_enum {
NonExhaustiveEnum::Unit => {}
NonExhaustiveEnum::Tuple(_) => {}
_ => {}
}
//~^^ some variants are not matched explicitly
} }
#[deny(non_exhaustive_omitted_patterns)] #[deny(non_exhaustive_omitted_patterns)]

View File

@ -198,6 +198,20 @@ help: you might want to use `let else` to handle the variant that isn't matched
LL | let local_refutable @ NonExhaustiveEnum::Unit = NonExhaustiveEnum::Unit else { todo!() }; LL | let local_refutable @ NonExhaustiveEnum::Unit = NonExhaustiveEnum::Unit else { todo!() };
| ++++++++++++++++ | ++++++++++++++++
error: aborting due to 9 previous errors; 6 warnings emitted error: some variants are not matched explicitly
--> $DIR/omitted-patterns.rs:202:9
|
LL | _ => {}
| ^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
|
= help: ensure that all variants are matched explicitly by adding the suggested match arms
= note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
note: the lint level is defined here
--> $DIR/omitted-patterns.rs:198:12
|
LL | #[deny(non_exhaustive_omitted_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 10 previous errors; 6 warnings emitted
For more information about this error, try `rustc --explain E0005`. For more information about this error, try `rustc --explain E0005`.