Skip to content

name resolution for guard patterns #140746

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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 compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ impl Pat {
/// Walk top-down and call `it` in each place where a pattern occurs
/// starting with the root pattern `walk` is called on. If `it` returns
/// false then we will descend no further but siblings will be processed.
pub fn walk(&self, it: &mut impl FnMut(&Pat) -> bool) {
pub fn walk<'ast>(&'ast self, it: &mut impl FnMut(&'ast Pat) -> bool) {
if !it(self) {
return;
}
Expand Down
129 changes: 95 additions & 34 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ enum PatBoundCtx {
Or,
}

/// Tracks bindings resolved within a pattern. This serves two purposes:
///
/// - This tracks when identifiers are bound multiple times within a pattern. In a product context,
/// this is an error. In an or-pattern, this lets us reuse the same resolution for each instance.
/// See `fresh_binding` and `resolve_pattern_inner` for more information.
///
/// - The guard expression of a guard pattern may use bindings from within the guard pattern, but
/// not from elsewhere in the pattern containing it. This allows us to isolate the bindings in the
/// subpattern to construct the scope for the guard.
///
/// Each identifier must map to at most one distinct [`Res`].
type PatternBindings = SmallVec<[(PatBoundCtx, FxIndexMap<Ident, Res>); 1]>;

/// Does this the item (from the item rib scope) allow generic parameters?
#[derive(Copy, Clone, Debug)]
pub(crate) enum HasGenericParams {
Expand Down Expand Up @@ -786,7 +799,14 @@ impl<'ra: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'r
fn visit_pat(&mut self, p: &'ast Pat) {
let prev = self.diag_metadata.current_pat;
self.diag_metadata.current_pat = Some(p);
visit::walk_pat(self, p);

if let PatKind::Guard(subpat, _) = &p.kind {
// We walk the guard expression in `resolve_pattern_inner`. Don't resolve it twice.
self.visit_pat(subpat);
} else {
visit::walk_pat(self, p);
}

self.diag_metadata.current_pat = prev;
}
fn visit_local(&mut self, local: &'ast Local) {
Expand Down Expand Up @@ -2297,7 +2317,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
fn resolve_fn_params(
&mut self,
has_self: bool,
inputs: impl Iterator<Item = (Option<&'ast Pat>, &'ast Ty)>,
inputs: impl Iterator<Item = (Option<&'ast Pat>, &'ast Ty)> + Clone,
) -> Result<LifetimeRes, (Vec<MissingLifetime>, Vec<ElisionFnParameter>)> {
enum Elision {
/// We have not found any candidate.
Expand All @@ -2319,15 +2339,20 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
let mut parameter_info = Vec::new();
let mut all_candidates = Vec::new();

// Resolve and apply bindings first so diagnostics can see if they're used in types.
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];
for (index, (pat, ty)) in inputs.enumerate() {
debug!(?pat, ?ty);
for (pat, _) in inputs.clone() {
debug!("resolving bindings in pat = {pat:?}");
self.with_lifetime_rib(LifetimeRibKind::Elided(LifetimeRes::Infer), |this| {
if let Some(pat) = pat {
this.resolve_pattern(pat, PatternSource::FnParam, &mut bindings);
}
});
}
self.apply_pattern_bindings(bindings);

for (index, (pat, ty)) in inputs.enumerate() {
debug!("resolving type for pat = {pat:?}, ty = {ty:?}");
// Record elision candidates only for this parameter.
debug_assert_matches!(self.lifetime_elision_candidates, None);
self.lifetime_elision_candidates = Some(Default::default());
Expand Down Expand Up @@ -3615,16 +3640,10 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
self.visit_path(&delegation.path, delegation.id);
let Some(body) = &delegation.body else { return };
self.with_rib(ValueNS, RibKind::FnOrCoroutine, |this| {
// `PatBoundCtx` is not necessary in this context
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];

let span = delegation.path.segments.last().unwrap().ident.span;
this.fresh_binding(
Ident::new(kw::SelfLower, span),
delegation.id,
PatternSource::FnParam,
&mut bindings,
);
let ident = Ident::new(kw::SelfLower, span.normalize_to_macro_rules());
let res = Res::Local(delegation.id);
this.innermost_rib_bindings(ValueNS).insert(ident, res);
this.visit_block(body);
});
}
Expand All @@ -3635,6 +3654,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
for Param { pat, .. } in params {
this.resolve_pattern(pat, PatternSource::FnParam, &mut bindings);
}
this.apply_pattern_bindings(bindings);
});
for Param { ty, .. } in params {
self.visit_ty(ty);
Expand Down Expand Up @@ -3851,13 +3871,32 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
fn resolve_pattern_top(&mut self, pat: &'ast Pat, pat_src: PatternSource) {
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];
self.resolve_pattern(pat, pat_src, &mut bindings);
self.apply_pattern_bindings(bindings);
}

/// Apply the bindings from a pattern to the innermost rib of the current scope.
fn apply_pattern_bindings(&mut self, mut pat_bindings: PatternBindings) {
let rib_bindings = self.innermost_rib_bindings(ValueNS);
let Some((_, pat_bindings)) = pat_bindings.pop() else {
bug!("tried applying nonexistent bindings from pattern");
};

if rib_bindings.is_empty() {
// Often, such as for match arms, the bindings are introduced into a new rib.
// In this case, we can move the bindings over directly.
*rib_bindings = pat_bindings;
} else {
rib_bindings.extend(pat_bindings);
}
}

/// Resolve bindings in a pattern. `apply_pattern_bindings` must be called after to introduce
/// the bindings into scope.
fn resolve_pattern(
&mut self,
pat: &'ast Pat,
pat_src: PatternSource,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
bindings: &mut PatternBindings,
) {
// We walk the pattern before declaring the pattern's inner bindings,
// so that we avoid resolving a literal expression to a binding defined
Expand Down Expand Up @@ -3890,9 +3929,9 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
#[tracing::instrument(skip(self, bindings), level = "debug")]
fn resolve_pattern_inner(
&mut self,
pat: &Pat,
pat: &'ast Pat,
pat_src: PatternSource,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
bindings: &mut PatternBindings,
) {
// Visit all direct subpatterns of this pattern.
pat.walk(&mut |pat| {
Expand Down Expand Up @@ -3950,6 +3989,27 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
// Prevent visiting `ps` as we've already done so above.
return false;
}
PatKind::Guard(ref subpat, ref guard) => {
// Add a new set of bindings to the stack to collect bindings in `subpat`.
bindings.push((PatBoundCtx::Product, Default::default()));
self.resolve_pattern_inner(subpat, pat_src, bindings);
// These bindings, but none from the surrounding pattern, are visible in the
// guard; put them in scope and resolve `guard`.
let subpat_bindings = bindings.pop().unwrap().1;
self.with_rib(ValueNS, RibKind::Normal, |this| {
*this.innermost_rib_bindings(ValueNS) = subpat_bindings.clone();
this.resolve_expr(guard, None);
});
// Propagate the subpattern's bindings upwards.
// FIXME(guard_patterns): For `if let` guards, we'll also need to get the
// bindings introduced by the guard from its rib and propagate them upwards.
// This will require checking the identifiers for overlaps with `bindings`, like
// what `fresh_binding` does (ideally sharing its logic). To keep them separate
// from `subpat_bindings`, we can introduce a fresh rib for the guard.
bindings.last_mut().unwrap().1.extend(subpat_bindings);
// Prevent visiting `subpat` as we've already done so above.
return false;
}
_ => {}
}
true
Expand Down Expand Up @@ -3988,20 +4048,17 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
ident: Ident,
pat_id: NodeId,
pat_src: PatternSource,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
bindings: &mut PatternBindings,
) -> Res {
// Add the binding to the local ribs, if it doesn't already exist in the bindings map.
// Add the binding to the bindings map, if it doesn't already exist.
// (We must not add it if it's in the bindings map because that breaks the assumptions
// later passes make about or-patterns.)
let ident = ident.normalize_to_macro_rules();

let mut bound_iter = bindings.iter().filter(|(_, set)| set.contains(&ident));
// Already bound in a product pattern? e.g. `(a, a)` which is not allowed.
let already_bound_and = bound_iter.clone().any(|(ctx, _)| *ctx == PatBoundCtx::Product);
// Already bound in an or-pattern? e.g. `V1(a) | V2(a)`.
// This is *required* for consistency which is checked later.
let already_bound_or = bound_iter.any(|(ctx, _)| *ctx == PatBoundCtx::Or);

let already_bound_and = bindings
.iter()
.any(|(ctx, map)| *ctx == PatBoundCtx::Product && map.contains_key(&ident));
if already_bound_and {
// Overlap in a product pattern somewhere; report an error.
use ResolutionError::*;
Expand All @@ -4014,19 +4071,23 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
self.report_error(ident.span, error(ident));
}

// Record as bound.
bindings.last_mut().unwrap().1.insert(ident);

if already_bound_or {
// Already bound in an or-pattern? e.g. `V1(a) | V2(a)`.
// This is *required* for consistency which is checked later.
let already_bound_or = bindings
.iter()
.find_map(|(ctx, map)| if *ctx == PatBoundCtx::Or { map.get(&ident) } else { None });
let res = if let Some(&res) = already_bound_or {
// `Variant1(a) | Variant2(a)`, ok
// Reuse definition from the first `a`.
self.innermost_rib_bindings(ValueNS)[&ident]
} else {
// A completely fresh binding is added to the set.
let res = Res::Local(pat_id);
self.innermost_rib_bindings(ValueNS).insert(ident, res);
res
}
} else {
// A completely fresh binding is added to the map.
Res::Local(pat_id)
};

// Record as bound.
bindings.last_mut().unwrap().1.insert(ident, res);
res
}

fn innermost_rib_bindings(&mut self, ns: Namespace) -> &mut FxIndexMap<Ident, Res> {
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/feature-gates/feature-gate-guard-patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ fn other_guards_dont() {

let ((x if guard(x)) | x) = 0;
//~^ ERROR: guard patterns are experimental
//~| ERROR: cannot find value `x`

if let (x if guard(x)) = 0 {}
//~^ ERROR: guard patterns are experimental
Expand All @@ -37,7 +36,6 @@ fn other_guards_dont() {

fn even_as_function_parameters(((x if guard(x), _) | (_, x)): (i32, i32)) {}
//~^ ERROR: guard patterns are experimental
//~| ERROR: cannot find value `x`

fn guard<T>(x: T) -> bool {
unimplemented!()
Expand Down
31 changes: 6 additions & 25 deletions tests/ui/feature-gates/feature-gate-guard-patterns.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,6 @@ LL - (0 if guard(0)) => {},
LL + 0 if guard(0) => {},
|

error[E0425]: cannot find value `x` in this scope
--> $DIR/feature-gate-guard-patterns.rs:23:22
|
LL | let ((x if guard(x)) | x) = 0;
| ^ not found in this scope

error[E0425]: cannot find value `x` in this scope
--> $DIR/feature-gate-guard-patterns.rs:38:45
|
LL | fn even_as_function_parameters(((x if guard(x), _) | (_, x)): (i32, i32)) {}
| ^
|
help: the binding `x` is available in a different scope in the same function
--> $DIR/feature-gate-guard-patterns.rs:23:11
|
LL | let ((x if guard(x)) | x) = 0;
| ^

error[E0658]: guard patterns are experimental
--> $DIR/feature-gate-guard-patterns.rs:18:15
|
Expand All @@ -51,7 +33,7 @@ LL | let ((x if guard(x)) | x) = 0;
= help: consider using match arm guards

error[E0658]: guard patterns are experimental
--> $DIR/feature-gate-guard-patterns.rs:27:18
--> $DIR/feature-gate-guard-patterns.rs:26:18
|
LL | if let (x if guard(x)) = 0 {}
| ^^^^^^^^
Expand All @@ -62,7 +44,7 @@ LL | if let (x if guard(x)) = 0 {}
= help: consider using match arm guards

error[E0658]: guard patterns are experimental
--> $DIR/feature-gate-guard-patterns.rs:30:21
--> $DIR/feature-gate-guard-patterns.rs:29:21
|
LL | while let (x if guard(x)) = 0 {}
| ^^^^^^^^
Expand All @@ -73,7 +55,7 @@ LL | while let (x if guard(x)) = 0 {}
= help: consider using match arm guards

error[E0658]: guard patterns are experimental
--> $DIR/feature-gate-guard-patterns.rs:34:21
--> $DIR/feature-gate-guard-patterns.rs:33:21
|
LL | while let (x if guard(x)) = 0 {}
| ^^^^^^^^
Expand All @@ -84,7 +66,7 @@ LL | while let (x if guard(x)) = 0 {}
= help: consider using match arm guards

error[E0658]: guard patterns are experimental
--> $DIR/feature-gate-guard-patterns.rs:38:39
--> $DIR/feature-gate-guard-patterns.rs:37:39
|
LL | fn even_as_function_parameters(((x if guard(x), _) | (_, x)): (i32, i32)) {}
| ^^^^^^^^
Expand All @@ -94,7 +76,6 @@ LL | fn even_as_function_parameters(((x if guard(x), _) | (_, x)): (i32, i32)) {
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
= help: consider using match arm guards

error: aborting due to 9 previous errors
error: aborting due to 7 previous errors

Some errors have detailed explanations: E0425, E0658.
For more information about an error, try `rustc --explain E0425`.
For more information about this error, try `rustc --explain E0658`.
Loading
Loading