Auto merge of rust-lang#140746 - dianne:guard-pat-res, r=<try>

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: rust-lang#129967

cc `@Nadrieril`

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -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;
         }

compiler/rustc_resolve/src/late.rs

@@ -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,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
@@ -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::*;
@@ -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> {

tests/ui/feature-gates/feature-gate-guard-patterns.rs

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

tests/ui/feature-gates/feature-gate-guard-patterns.stderr

@@ -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`.

tests/ui/pattern/rfc-3637-guard-patterns/name-resolution.rs

@@ -0,0 +1,66 @@
+//! 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
+    }
+}