Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ICE for broken or-pattern in async fn #71557

Merged
merged 1 commit into from
May 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc_mir_build/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}));

debug!("ast_block_stmts: pattern={:?}", pattern);
this.visit_bindings(
this.visit_primary_bindings(
&pattern,
UserTypeProjections::none(),
&mut |this, _, _, _, node, span, _, _| {
Expand Down
59 changes: 43 additions & 16 deletions src/librustc_mir_build/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
opt_match_place: Option<(Option<&Place<'tcx>>, Span)>,
) -> Option<SourceScope> {
debug!("declare_bindings: pattern={:?}", pattern);
self.visit_bindings(
self.visit_primary_bindings(
&pattern,
UserTypeProjections::none(),
&mut |this, mutability, name, mode, var, span, ty, user_ty| {
Expand Down Expand Up @@ -563,7 +563,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.schedule_drop(span, region_scope, local_id, DropKind::Value);
}

pub(super) fn visit_bindings(
/// Visit all of the primary bindings in a patterns, that is, visit the
/// leftmost occurrence of each variable bound in a pattern. A variable
/// will occur more than once in an or-pattern.
pub(super) fn visit_primary_bindings(
&mut self,
pattern: &Pat<'tcx>,
pattern_user_ty: UserTypeProjections,
Expand All @@ -578,12 +581,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
UserTypeProjections,
),
) {
debug!("visit_bindings: pattern={:?} pattern_user_ty={:?}", pattern, pattern_user_ty);
debug!(
"visit_primary_bindings: pattern={:?} pattern_user_ty={:?}",
pattern, pattern_user_ty
);
match *pattern.kind {
PatKind::Binding { mutability, name, mode, var, ty, ref subpattern, .. } => {
f(self, mutability, name, mode, var, pattern.span, ty, pattern_user_ty.clone());
PatKind::Binding {
mutability,
name,
mode,
var,
ty,
ref subpattern,
is_primary,
..
} => {
if is_primary {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
f(self, mutability, name, mode, var, pattern.span, ty, pattern_user_ty.clone());
}
if let Some(subpattern) = subpattern.as_ref() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't subpatterns only relevant for the primary, too, as each variable must occur in all variants?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of this PR is that we can't assume that patterns are well formed because the error isn't fatal.

self.visit_bindings(subpattern, pattern_user_ty, f);
self.visit_primary_bindings(subpattern, pattern_user_ty, f);
}
}

Expand All @@ -592,20 +609,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let from = u32::try_from(prefix.len()).unwrap();
let to = u32::try_from(suffix.len()).unwrap();
for subpattern in prefix {
self.visit_bindings(subpattern, pattern_user_ty.clone().index(), f);
self.visit_primary_bindings(subpattern, pattern_user_ty.clone().index(), f);
}
for subpattern in slice {
self.visit_bindings(subpattern, pattern_user_ty.clone().subslice(from, to), f);
self.visit_primary_bindings(
subpattern,
pattern_user_ty.clone().subslice(from, to),
f,
);
}
for subpattern in suffix {
self.visit_bindings(subpattern, pattern_user_ty.clone().index(), f);
self.visit_primary_bindings(subpattern, pattern_user_ty.clone().index(), f);
}
}

PatKind::Constant { .. } | PatKind::Range { .. } | PatKind::Wild => {}

PatKind::Deref { ref subpattern } => {
self.visit_bindings(subpattern, pattern_user_ty.deref(), f);
self.visit_primary_bindings(subpattern, pattern_user_ty.deref(), f);
}

PatKind::AscribeUserType {
Expand All @@ -630,26 +651,32 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
projs: Vec::new(),
};
let subpattern_user_ty = pattern_user_ty.push_projection(&projection, user_ty_span);
self.visit_bindings(subpattern, subpattern_user_ty, f)
self.visit_primary_bindings(subpattern, subpattern_user_ty, f)
}

PatKind::Leaf { ref subpatterns } => {
for subpattern in subpatterns {
let subpattern_user_ty = pattern_user_ty.clone().leaf(subpattern.field);
debug!("visit_bindings: subpattern_user_ty={:?}", subpattern_user_ty);
self.visit_bindings(&subpattern.pattern, subpattern_user_ty, f);
debug!("visit_primary_bindings: subpattern_user_ty={:?}", subpattern_user_ty);
self.visit_primary_bindings(&subpattern.pattern, subpattern_user_ty, f);
}
}

PatKind::Variant { adt_def, substs: _, variant_index, ref subpatterns } => {
for subpattern in subpatterns {
let subpattern_user_ty =
pattern_user_ty.clone().variant(adt_def, variant_index, subpattern.field);
self.visit_bindings(&subpattern.pattern, subpattern_user_ty, f);
self.visit_primary_bindings(&subpattern.pattern, subpattern_user_ty, f);
}
}
PatKind::Or { ref pats } => {
self.visit_bindings(&pats[0], pattern_user_ty, f);
// In cases where we recover from errors the primary bindings
// may not all be in the leftmost subpattern. For example in
// `let (x | y) = ...`, the primary binding of `y` occurs in
// the right subpattern
for subpattern in pats {
self.visit_primary_bindings(subpattern, pattern_user_ty.clone(), f);
}
}
}
}
Expand Down Expand Up @@ -1953,7 +1980,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
is_block_tail: None,
local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
binding_mode,
// hypothetically, `visit_bindings` could try to unzip
// hypothetically, `visit_primary_bindings` could try to unzip
// an outermost hir::Ty as we descend, matching up
// idents in pat; but complex w/ unclear UI payoff.
// Instead, just abandon providing diagnostic info.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Ok(())
}

PatKind::Binding { name, mutability, mode, var, ty, ref subpattern } => {
PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => {
candidate.bindings.push(Binding {
name,
mutability,
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_mir_build/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ crate enum PatKind<'tcx> {
var: hir::HirId,
ty: Ty<'tcx>,
subpattern: Option<Pat<'tcx>>,
/// Is this the leftmost occurance of the binding, i.e., is `var` the
/// `HirId` of this pattern?
is_primary: bool,
},

/// `Foo(...)` or `Foo{...}` or `Foo`, where `Foo` is a variant name from an ADT with
Expand Down Expand Up @@ -601,6 +604,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
var: id,
ty: var_ty,
subpattern: self.lower_opt_pattern(sub),
is_primary: id == pat.hir_id,
}
}

Expand Down Expand Up @@ -964,14 +968,15 @@ impl<'tcx> PatternFoldable<'tcx> for PatKind<'tcx> {
user_ty_span,
},
},
PatKind::Binding { mutability, name, mode, var, ty, ref subpattern } => {
PatKind::Binding { mutability, name, mode, var, ty, ref subpattern, is_primary } => {
PatKind::Binding {
mutability: mutability.fold_with(folder),
name: name.fold_with(folder),
mode: mode.fold_with(folder),
var: var.fold_with(folder),
ty: ty.fold_with(folder),
subpattern: subpattern.fold_with(folder),
is_primary,
}
}
PatKind::Variant { adt_def, substs, variant_index, ref subpatterns } => {
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/or-patterns/mismatched-bindings-async-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Regression test for #71297
// edition:2018

#![feature(or_patterns)]

async fn a((x | s): String) {}
//~^ ERROR variable `x` is not bound in all patterns
//~| ERROR variable `s` is not bound in all patterns

async fn b() {
let x | s = String::new();
//~^ ERROR variable `x` is not bound in all patterns
//~| ERROR variable `s` is not bound in all patterns
}

fn main() {}
35 changes: 35 additions & 0 deletions src/test/ui/or-patterns/mismatched-bindings-async-fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error[E0408]: variable `x` is not bound in all patterns
--> $DIR/mismatched-bindings-async-fn.rs:6:17
|
LL | async fn a((x | s): String) {}
| - ^ pattern doesn't bind `x`
| |
| variable not in all patterns

error[E0408]: variable `s` is not bound in all patterns
--> $DIR/mismatched-bindings-async-fn.rs:6:13
|
LL | async fn a((x | s): String) {}
| ^ - variable not in all patterns
| |
| pattern doesn't bind `s`

error[E0408]: variable `x` is not bound in all patterns
--> $DIR/mismatched-bindings-async-fn.rs:11:13
|
LL | let x | s = String::new();
| - ^ pattern doesn't bind `x`
| |
| variable not in all patterns

error[E0408]: variable `s` is not bound in all patterns
--> $DIR/mismatched-bindings-async-fn.rs:11:9
|
LL | let x | s = String::new();
| ^ - variable not in all patterns
| |
| pattern doesn't bind `s`

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0408`.