Rollup merge of #140746 - dianne:guard-pat-res, r=oli-obk

name resolution for guard patterns

This PR provides an initial implementation of name resolution for guard patterns [(RFC 3637)](https://github.com/rust-lang/rfcs/blob/master/text/3637-guard-patterns.md). This does not change the requirement that the bindings on either side of an or-pattern must be the same [(proposal here)](https://github.com/rust-lang/rfcs/blob/master/text/3637-guard-patterns.md#allowing-mismatching-bindings-when-possible); the code that handles that is separate from what this PR touches, so I'm saving it for a follow-up.

On a technical level, this separates "collecting the bindings in a pattern" (which was already done for or-patterns) from "introducing those bindings into scope". I believe the approach used here can be extended straightforwardly in the future to work with `if let` guard patterns, but I haven't tried it myself since we don't allow those yet.

Tracking issue for guard patterns: #129967

cc ``@Nadrieril``
This commit is contained in:
León Orell Valerian Liehr 2025-05-18 18:44:11 +02:00 committed by GitHub
commit 4e5b1aa055
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 320 additions and 62 deletions

View file

@ -610,7 +610,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;
}

View file

@ -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 {
@ -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) {
@ -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.
@ -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());
@ -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);
});
}
@ -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);
@ -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
@ -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| {
@ -3950,6 +3989,31 @@ 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()));
// Resolving `subpat` adds bindings onto the newly-pushed context. After, the
// total number of contexts on the stack should be the same as before.
let binding_ctx_stack_len = bindings.len();
self.resolve_pattern_inner(subpat, pat_src, bindings);
assert_eq!(bindings.len(), binding_ctx_stack_len);
// 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
@ -3988,20 +4052,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::*;
@ -4014,19 +4075,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> {

View file

@ -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
@ -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!()

View file

@ -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
|
@ -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 {}
| ^^^^^^^^
@ -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 {}
| ^^^^^^^^
@ -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 {}
| ^^^^^^^^
@ -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)) {}
| ^^^^^^^^
@ -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`.

View file

@ -0,0 +1,81 @@
//! Test that guard patterns can see bindings already in scope and bindings introduced in their
//! subpattern, but no other bindings from the containing pattern. Also make sure bindings
//! introduced in guard patterns are visible in fn/arm/loop/etc bodies.
#![feature(guard_patterns)]
#![expect(incomplete_features)]
fn good_fn_item(((x if x) | x): bool) -> bool { x }
fn bad_fn_item_1(x: bool, ((y if x) | y): bool) {}
//~^ ERROR cannot find value `x` in this scope
fn bad_fn_item_2(((x if y) | x): bool, y: bool) {}
//~^ ERROR cannot find value `y` in this scope
fn main() {
let ((local if local) if local) = false;
match (true, true) {
(x if local, y if good_fn_item(y)) => x && y,
(x, y if x) => x && y,
//~^ ERROR cannot find value `x` in this scope
(x if y, y) => x && y,
//~^ ERROR cannot find value `y` in this scope
};
match (true,) {
(x @ y if x && y,) => x && y,
(x @ (y if y),) => x && y,
(x @ (y if x),) => x && y,
//~^ ERROR cannot find value `x` in this scope
};
match (Ok(true),) {
((Ok(x) | Err(x)) if good_fn_item(x),) => x,
((Ok(x) if local) | (Err(x) if good_fn_item(x)),) => x,
((Ok(x if x) if x) | (Err(x if x) if x) if x,) if x => x,
((Ok(x) if y) | (Err(y) if x),) => x && y,
//~^ ERROR variable `x` is not bound in all patterns
//~| ERROR variable `y` is not bound in all patterns
//~| ERROR cannot find value `x` in this scope
//~| ERROR cannot find value `y` in this scope
};
let (_ if nonexistent) = true;
//~^ ERROR cannot find value `nonexistent` in this scope
if let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
//~^ ERROR cannot find value `x` in this scope
//~| ERROR cannot find value `y` in this scope
while let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
//~^ ERROR cannot find value `x` in this scope
//~| ERROR cannot find value `y` in this scope
for ((x, y if x) | (x if y, y)) in [(true, true)] { x && y; }
//~^ ERROR cannot find value `x` in this scope
//~| ERROR cannot find value `y` in this scope
(|(x if x), (y if y)| x && y)(true, true);
(|(x if y), (y if x)| x && y)(true, true);
//~^ ERROR cannot find value `x` in this scope
//~| ERROR cannot find value `y` in this scope
// FIXME(guard_patterns): mismatched bindings are not yet allowed
match Some(0) {
Some(x if x > 0) | None => {}
//~^ ERROR variable `x` is not bound in all patterns
}
}
/// Make sure shadowing is handled properly. In particular, if a pattern shadows an identifier,
/// a guard pattern's guard should still see the original binding if the shadowing binding isn't in
/// its subpattern.
fn test_shadowing(local: bool) -> u8 {
match (0, 0) {
// The `local` binding here shadows the `bool` definition, so we get a type error.
//~v ERROR mismatched types
local if local => 0,
// The guards here should see the `bool` definition of `local`, not the new `u8` binding.
// The body should see the new binding.
(local, _ if local) => local,
(_ if local, local) => local,
}
}

View file

@ -0,0 +1,133 @@
error[E0408]: variable `y` is not bound in all patterns
--> $DIR/name-resolution.rs:37:10
|
LL | ((Ok(x) if y) | (Err(y) if x),) => x && y,
| ^^^^^^^^^^^^ - variable not in all patterns
| |
| pattern doesn't bind `y`
error[E0408]: variable `x` is not bound in all patterns
--> $DIR/name-resolution.rs:37:25
|
LL | ((Ok(x) if y) | (Err(y) if x),) => x && y,
| - ^^^^^^^^^^^^^ pattern doesn't bind `x`
| |
| variable not in all patterns
error[E0408]: variable `x` is not bound in all patterns
--> $DIR/name-resolution.rs:63:28
|
LL | Some(x if x > 0) | None => {}
| - ^^^^ pattern doesn't bind `x`
| |
| variable not in all patterns
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:10:34
|
LL | fn bad_fn_item_1(x: bool, ((y if x) | y): bool) {}
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:12:25
|
LL | fn bad_fn_item_2(((x if y) | x): bool, y: bool) {}
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:20:18
|
LL | (x, y if x) => x && y,
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:22:15
|
LL | (x if y, y) => x && y,
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:29:20
|
LL | (x @ (y if x),) => x && y,
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:37:20
|
LL | ((Ok(x) if y) | (Err(y) if x),) => x && y,
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:37:36
|
LL | ((Ok(x) if y) | (Err(y) if x),) => x && y,
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `nonexistent` in this scope
--> $DIR/name-resolution.rs:44:15
|
LL | let (_ if nonexistent) = true;
| ^^^^^^^^^^^ not found in this scope
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:46:22
|
LL | if let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:46:33
|
LL | if let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:49:25
|
LL | while let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:49:36
|
LL | while let ((x, y if x) | (x if y, y)) = (true, true) { x && y; }
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:52:19
|
LL | for ((x, y if x) | (x if y, y)) in [(true, true)] { x && y; }
| ^ help: a local variable with a similar name exists: `y`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:52:30
|
LL | for ((x, y if x) | (x if y, y)) in [(true, true)] { x && y; }
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `y` in this scope
--> $DIR/name-resolution.rs:57:13
|
LL | (|(x if y), (y if x)| x && y)(true, true);
| ^ help: a local variable with a similar name exists: `x`
error[E0425]: cannot find value `x` in this scope
--> $DIR/name-resolution.rs:57:23
|
LL | (|(x if y), (y if x)| x && y)(true, true);
| ^ help: a local variable with a similar name exists: `y`
error[E0308]: mismatched types
--> $DIR/name-resolution.rs:75:18
|
LL | local if local => 0,
| ^^^^^ expected `bool`, found `({integer}, {integer})`
|
= note: expected type `bool`
found tuple `({integer}, {integer})`
error: aborting due to 20 previous errors
Some errors have detailed explanations: E0308, E0408, E0425.
For more information about an error, try `rustc --explain E0308`.