From 83e1efb254d3de48c4c00eed67698a4d10173407 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 10 Jul 2024 16:15:23 +1000 Subject: [PATCH] Replace a long inline "autoref" comment with method docs This comment has two problems: - It is very long, making the flow of the enclosing method hard to follow. - It starts by talking about an `autoref` flag that hasn't existed since #59114. This PR therefore replaces the long inline comment with a revised doc comment on `bind_matched_candidate_for_guard`, and some shorter inline comments. For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the `autoref` flag. --- .../rustc_mir_build/src/build/matches/mod.rs | 165 +++++++++--------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 5695c881ecc..7b92c0a1be8 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2177,92 +2177,15 @@ fn bind_and_guard_matched_candidate<'pat>( self.ascribe_types(block, ascriptions); - // rust-lang/rust#27282: The `autoref` business deserves some - // explanation here. - // - // The intent of the `autoref` flag is that when it is true, - // then any pattern bindings of type T will map to a `&T` - // within the context of the guard expression, but will - // continue to map to a `T` in the context of the arm body. To - // avoid surfacing this distinction in the user source code - // (which would be a severe change to the language and require - // far more revision to the compiler), when `autoref` is true, - // then any occurrence of the identifier in the guard - // expression will automatically get a deref op applied to it. - // - // So an input like: - // - // ``` - // let place = Foo::new(); - // match place { foo if inspect(foo) - // => feed(foo), ... } - // ``` - // - // will be treated as if it were really something like: - // - // ``` - // let place = Foo::new(); - // match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) } - // => { let tmp2 = place; feed(tmp2) }, ... } - // ``` - // - // And an input like: - // - // ``` - // let place = Foo::new(); - // match place { ref mut foo if inspect(foo) - // => feed(foo), ... } - // ``` - // - // will be treated as if it were really something like: - // - // ``` - // let place = Foo::new(); - // match place { Foo { .. } if { let tmp1 = & &mut place; inspect(*tmp1) } - // => { let tmp2 = &mut place; feed(tmp2) }, ... } - // ``` - // - // In short, any pattern binding will always look like *some* - // kind of `&T` within the guard at least in terms of how the - // MIR-borrowck views it, and this will ensure that guard - // expressions cannot mutate their the match inputs via such - // bindings. (It also ensures that guard expressions can at - // most *copy* values from such bindings; non-Copy things - // cannot be moved via pattern bindings in guard expressions.) - // - // ---- - // - // Implementation notes (under assumption `autoref` is true). - // - // To encode the distinction above, we must inject the - // temporaries `tmp1` and `tmp2`. - // - // There are two cases of interest: binding by-value, and binding by-ref. - // - // 1. Binding by-value: Things are simple. - // - // * Establishing `tmp1` creates a reference into the - // matched place. This code is emitted by - // bind_matched_candidate_for_guard. - // - // * `tmp2` is only initialized "lazily", after we have - // checked the guard. Thus, the code that can trigger - // moves out of the candidate can only fire after the - // guard evaluated to true. This initialization code is - // emitted by bind_matched_candidate_for_arm. - // - // 2. Binding by-reference: Things are tricky. - // - // * Here, the guard expression wants a `&&` or `&&mut` - // into the original input. This means we need to borrow - // the reference that we create for the arm. - // * So we eagerly create the reference for the arm and then take a - // reference to that. + // Lower an instance of the arm guard (if present) for this candidate, + // and then perform bindings for the arm body. if let Some((arm, match_scope)) = arm_match_scope && let Some(guard) = arm.guard { let tcx = self.tcx; + // Bindings for guards require some extra handling to automatically + // insert implicit references/dereferences. self.bind_matched_candidate_for_guard(block, schedule_drops, bindings.clone()); let guard_frame = GuardFrame { locals: bindings.clone().map(|b| GuardFrameLocal::new(b.var_id)).collect(), @@ -2402,6 +2325,82 @@ fn ascribe_types( } } + /// Binding for guards is a bit different from binding for the arm body, + /// because an extra layer of implicit reference/dereference is added. + /// + /// The idea is that any pattern bindings of type T will map to a `&T` within + /// the context of the guard expression, but will continue to map to a `T` + /// in the context of the arm body. To avoid surfacing this distinction in + /// the user source code (which would be a severe change to the language and + /// require far more revision to the compiler), any occurrence of the + /// identifier in the guard expression will automatically get a deref op + /// applied to it. (See the caller of [`Self::is_bound_var_in_guard`].) + /// + /// So an input like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { foo if inspect(foo) + /// => feed(foo), ... } + /// ``` + /// + /// will be treated as if it were really something like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) } + /// => { let tmp2 = place; feed(tmp2) }, ... } + /// ``` + /// + /// And an input like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { ref mut foo if inspect(foo) + /// => feed(foo), ... } + /// ``` + /// + /// will be treated as if it were really something like: + /// + /// ```ignore (illustrative) + /// let place = Foo::new(); + /// match place { Foo { .. } if { let tmp1 = & &mut place; inspect(*tmp1) } + /// => { let tmp2 = &mut place; feed(tmp2) }, ... } + /// ``` + /// --- + /// + /// ## Implementation notes + /// + /// To encode the distinction above, we must inject the + /// temporaries `tmp1` and `tmp2`. + /// + /// There are two cases of interest: binding by-value, and binding by-ref. + /// + /// 1. Binding by-value: Things are simple. + /// + /// * Establishing `tmp1` creates a reference into the + /// matched place. This code is emitted by + /// [`Self::bind_matched_candidate_for_guard`]. + /// + /// * `tmp2` is only initialized "lazily", after we have + /// checked the guard. Thus, the code that can trigger + /// moves out of the candidate can only fire after the + /// guard evaluated to true. This initialization code is + /// emitted by [`Self::bind_matched_candidate_for_arm_body`]. + /// + /// 2. Binding by-reference: Things are tricky. + /// + /// * Here, the guard expression wants a `&&` or `&&mut` + /// into the original input. This means we need to borrow + /// the reference that we create for the arm. + /// * So we eagerly create the reference for the arm and then take a + /// reference to that. + /// + /// --- + /// + /// See these PRs for some historical context: + /// - (introduction of autoref) + /// - (always use autoref) fn bind_matched_candidate_for_guard<'b>( &mut self, block: BasicBlock, @@ -2433,10 +2432,13 @@ fn bind_matched_candidate_for_guard<'b>( ); match binding.binding_mode.0 { ByRef::No => { + // The arm binding will be by value, so for the guard binding + // just take a shared reference to the matched place. let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, binding.source); self.cfg.push_assign(block, source_info, ref_for_guard, rvalue); } ByRef::Yes(mutbl) => { + // The arm binding will be by reference, so eagerly create it now. let value_for_arm = self.storage_live_binding( block, binding.var_id, @@ -2448,6 +2450,7 @@ fn bind_matched_candidate_for_guard<'b>( let rvalue = Rvalue::Ref(re_erased, util::ref_pat_borrow_kind(mutbl), binding.source); self.cfg.push_assign(block, source_info, value_for_arm, rvalue); + // For the guard binding, take a shared reference to that reference. let rvalue = Rvalue::Ref(re_erased, BorrowKind::Shared, value_for_arm); self.cfg.push_assign(block, source_info, ref_for_guard, rvalue); }