Auto merge of #148706 - dianne:slightly-lazier-temporary-scoping, r=<try>

compute temporary scopes when building MIR, not THIR

Author: rust-bors[bot]

compiler/rustc_hir_analysis/src/check/region.rs

@@ -516,12 +516,10 @@ fn resolve_local<'tcx>(
 
         if let Some(pat) = pat {
             if is_binding_pat(pat) {
-                visitor.scope_tree.record_rvalue_candidate(
-                    expr.hir_id,
-                    RvalueCandidate {
-                        target: expr.hir_id.local_id,
-                        lifetime: visitor.cx.var_parent,
-                    },
+                record_subexpr_extended_temp_scopes(
+                    &mut visitor.scope_tree,
+                    expr,
+                    visitor.cx.var_parent,
                 );
             }
         }
@@ -604,7 +602,7 @@ fn resolve_local<'tcx>(
         }
     }
 
-    /// If `expr` matches the `E&` grammar, then records an extended rvalue scope as appropriate:
+    /// If `expr` matches the `E&` grammar, then records an extended temporary scope as appropriate:
     ///
     /// ```text
     ///     E& = & ET
@@ -627,10 +625,7 @@ fn resolve_local<'tcx>(
         match expr.kind {
             hir::ExprKind::AddrOf(_, _, subexpr) => {
                 record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
-                visitor.scope_tree.record_rvalue_candidate(
-                    subexpr.hir_id,
-                    RvalueCandidate { target: subexpr.hir_id.local_id, lifetime: blk_id },
-                );
+                record_subexpr_extended_temp_scopes(&mut visitor.scope_tree, subexpr, blk_id);
             }
             hir::ExprKind::Struct(_, fields, _) => {
                 for field in fields {
@@ -687,6 +682,53 @@ fn resolve_local<'tcx>(
     }
 }
 
+/// Applied to an expression `expr` if `expr` -- or something owned or partially owned by
+/// `expr` -- is going to be indirectly referenced by a variable in a let statement. In that
+/// case, the "temporary lifetime" of `expr` is extended to be the block enclosing the `let`
+/// statement.
+///
+/// More formally, if `expr` matches the grammar `ET`, record the temporary scope of the matching
+/// `<rvalue>` as `lifetime`:
+///
+/// ```text
+///     ET = *ET
+///        | ET[...]
+///        | ET.f
+///        | (ET)
+///        | <rvalue>
+/// ```
+///
+/// Note: ET is intended to match "rvalues or places based on rvalues".
+fn record_subexpr_extended_temp_scopes(
+    scope_tree: &mut ScopeTree,
+    mut expr: &hir::Expr<'_>,
+    lifetime: Option<Scope>,
+) {
+    debug!(?expr, ?lifetime);
+
+    loop {
+        // Note: give all the expressions matching `ET` with the
+        // extended temporary lifetime, not just the innermost rvalue,
+        // because in MIR building if we must compile e.g., `*rvalue()`
+        // into a temporary, we request the temporary scope of the
+        // outer expression.
+
+        scope_tree.record_extended_temp_scope(expr.hir_id.local_id, lifetime);
+
+        match expr.kind {
+            hir::ExprKind::AddrOf(_, _, subexpr)
+            | hir::ExprKind::Unary(hir::UnOp::Deref, subexpr)
+            | hir::ExprKind::Field(subexpr, _)
+            | hir::ExprKind::Index(subexpr, _, _) => {
+                expr = subexpr;
+            }
+            _ => {
+                return;
+            }
+        }
+    }
+}
+
 impl<'tcx> ScopeResolutionVisitor<'tcx> {
     /// Records the current parent (if any) as the parent of `child_scope`.
     fn record_child_scope(&mut self, child_scope: Scope) {

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -40,7 +40,7 @@ use tracing::{debug, instrument};
 use crate::callee::{self, DeferredCallResolution};
 use crate::errors::{self, CtorIsPrivate};
 use crate::method::{self, MethodCallee};
-use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy, rvalue_scopes};
+use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy};
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// Produces warning on the given node, if the current point in the
@@ -604,13 +604,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.normalize(span, field.ty(self.tcx, args))
     }
 
-    pub(crate) fn resolve_rvalue_scopes(&self, def_id: DefId) {
-        let scope_tree = self.tcx.region_scope_tree(def_id);
-        let rvalue_scopes = { rvalue_scopes::resolve_rvalue_scopes(self, scope_tree, def_id) };
-        let mut typeck_results = self.typeck_results.borrow_mut();
-        typeck_results.rvalue_scopes = rvalue_scopes;
-    }
-
     /// Drain all obligations that are stalled on coroutines defined in this body.
     #[instrument(level = "debug", skip(self))]
     pub(crate) fn drain_stalled_coroutine_obligations(&self) {

compiler/rustc_hir_typeck/src/lib.rs

@@ -37,7 +37,6 @@ mod op;
 mod opaque_types;
 mod pat;
 mod place_op;
-mod rvalue_scopes;
 mod typeck_root_ctxt;
 mod upvar;
 mod writeback;
@@ -237,9 +236,6 @@ fn typeck_with_inspect<'tcx>(
     // because they don't constrain other type variables.
     fcx.closure_analyze(body);
     assert!(fcx.deferred_call_resolutions.borrow().is_empty());
-    // Before the coroutine analysis, temporary scopes shall be marked to provide more
-    // precise information on types to be captured.
-    fcx.resolve_rvalue_scopes(def_id.to_def_id());
 
     for (ty, span, code) in fcx.deferred_sized_obligations.borrow_mut().drain(..) {
         let ty = fcx.normalize(span, ty);

compiler/rustc_hir_typeck/src/rvalue_scopes.rs

@@ -79,9 +79,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         wbcx.visit_offset_of_container_types();
         wbcx.visit_potentially_region_dependent_goals();
 
-        wbcx.typeck_results.rvalue_scopes =
-            mem::take(&mut self.typeck_results.borrow_mut().rvalue_scopes);
-
         let used_trait_imports =
             mem::take(&mut self.typeck_results.borrow_mut().used_trait_imports);
         debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports);

compiler/rustc_hir_typeck/src/writeback.rs

@@ -11,7 +11,7 @@ use std::fmt;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::unord::UnordMap;
 use rustc_hir as hir;
-use rustc_hir::{HirId, HirIdMap, Node};
+use rustc_hir::{HirId, ItemLocalMap, Node};
 use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_span::{DUMMY_SP, Span};
 use tracing::debug;
@@ -221,27 +221,28 @@ pub struct ScopeTree {
     /// variable is declared.
     var_map: FxIndexMap<hir::ItemLocalId, Scope>,
 
-    /// Identifies expressions which, if captured into a temporary, ought to
-    /// have a temporary whose lifetime extends to the end of the enclosing *block*,
-    /// and not the enclosing *statement*. Expressions that are not present in this
-    /// table are not rvalue candidates. The set of rvalue candidates is computed
-    /// during type check based on a traversal of the AST.
-    pub rvalue_candidates: HirIdMap<RvalueCandidate>,
+    /// Tracks expressions with extended temporary scopes, based on the syntactic rules for
+    /// temporary lifetime extension. Further details may be found in
+    /// `rustc_hir_analysis::check::region` and in the [Reference].
+    ///
+    /// [Reference]: https://doc.rust-lang.org/nightly/reference/destructors.html#temporary-lifetime-extension
+    extended_temp_scopes: ItemLocalMap<Option<Scope>>,
 
     /// Backwards incompatible scoping that will be introduced in future editions.
     /// This information is used later for linting to identify locals and
     /// temporary values that will receive backwards-incompatible drop orders.
     pub backwards_incompatible_scope: UnordMap<hir::ItemLocalId, Scope>,
 }
 
-/// See the `rvalue_candidates` field for more information on rvalue
-/// candidates in general.
-/// The `lifetime` field is None to indicate that certain expressions escape
-/// into 'static and should have no local cleanup scope.
-#[derive(Debug, Copy, Clone, HashStable)]
-pub struct RvalueCandidate {
-    pub target: hir::ItemLocalId,
-    pub lifetime: Option<Scope>,
+/// Temporary lifetime information for expressions, used when lowering to MIR.
+#[derive(Clone, Copy, Debug, HashStable)]
+pub struct TempLifetime {
+    /// The scope in which a temporary should be dropped. If `None`, no drop is scheduled; this is
+    /// the case for lifetime-extended temporaries extended by a const/static item or const block.
+    pub temp_lifetime: Option<Scope>,
+    /// If `Some(lt)`, indicates that the lifetime of this temporary will change to `lt` in a future edition.
+    /// If `None`, then no changes are expected, or lints are disabled.
+    pub backwards_incompatible: Option<Scope>,
 }
 
 impl ScopeTree {
@@ -260,12 +261,13 @@ impl ScopeTree {
         self.var_map.insert(var, lifetime);
     }
 
-    pub fn record_rvalue_candidate(&mut self, var: HirId, candidate: RvalueCandidate) {
-        debug!("record_rvalue_candidate(var={var:?}, candidate={candidate:?})");
-        if let Some(lifetime) = &candidate.lifetime {
-            assert!(var.local_id != lifetime.local_id)
+    /// Make an association between a sub-expression and an extended lifetime
+    pub fn record_extended_temp_scope(&mut self, var: hir::ItemLocalId, lifetime: Option<Scope>) {
+        debug!(?var, ?lifetime);
+        if let Some(lifetime) = lifetime {
+            assert!(var != lifetime.local_id);
         }
-        self.rvalue_candidates.insert(var, candidate);
+        self.extended_temp_scopes.insert(var, lifetime);
     }
 
     /// Returns the narrowest scope that encloses `id`, if any.
@@ -337,4 +339,20 @@ impl ScopeTree {
 
         span_bug!(ty::tls::with(|tcx| inner.span(tcx, self)), "no enclosing temporary scope")
     }
+
+    /// Returns the scope when the temp created by `expr_id` will be cleaned up.
+    /// It also emits a lint on potential backwards incompatible change to the temporary scope
+    /// which is *for now* always shortening.
+    pub fn temporary_scope(&self, expr_id: hir::ItemLocalId) -> TempLifetime {
+        // Check for a designated extended temporary scope.
+        if let Some(&s) = self.extended_temp_scopes.get(&expr_id) {
+            debug!("temporary_scope({expr_id:?}) = {s:?} [custom]");
+            return TempLifetime { temp_lifetime: s, backwards_incompatible: None };
+        }
+
+        // Otherwise, locate the innermost terminating scope.
+        let (scope, backwards_incompatible) =
+            self.default_temporary_scope(Scope { local_id: expr_id, data: ScopeData::Node });
+        TempLifetime { temp_lifetime: Some(scope), backwards_incompatible }
+    }
 }

compiler/rustc_middle/src/middle/region.rs

@@ -256,25 +256,15 @@ pub struct Expr<'tcx> {
     /// The type of this expression
     pub ty: Ty<'tcx>,
 
-    /// The lifetime of this expression if it should be spilled into a
-    /// temporary
-    pub temp_lifetime: TempLifetime,
+    /// The id of the HIR expression whose [temporary scope] should be used for this expression.
+    ///
+    /// [temporary scope]: https://doc.rust-lang.org/reference/destructors.html#temporary-scopes
+    pub temp_scope_id: hir::ItemLocalId,
 
     /// span of the expression in the source
     pub span: Span,
 }
 
-/// Temporary lifetime information for THIR expressions
-#[derive(Clone, Copy, Debug, HashStable)]
-pub struct TempLifetime {
-    /// Lifetime for temporaries as expected.
-    /// This should be `None` in a constant context.
-    pub temp_lifetime: Option<region::Scope>,
-    /// If `Some(lt)`, indicates that the lifetime of this temporary will change to `lt` in a future edition.
-    /// If `None`, then no changes are expected, or lints are disabled.
-    pub backwards_incompatible: Option<region::Scope>,
-}
-
 #[derive(Clone, Debug, HashStable)]
 pub enum ExprKind<'tcx> {
     /// `Scope`s are used to explicitly mark destruction scopes,
@@ -1127,7 +1117,7 @@ mod size_asserts {
     use super::*;
     // tidy-alphabetical-start
     static_assert_size!(Block, 48);
-    static_assert_size!(Expr<'_>, 72);
+    static_assert_size!(Expr<'_>, 64);
     static_assert_size!(ExprKind<'_>, 40);
     static_assert_size!(Pat<'_>, 64);
     static_assert_size!(PatKind<'_>, 48);

compiler/rustc_middle/src/thir.rs

@@ -45,7 +45,7 @@ pub fn walk_expr<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
     expr: &'thir Expr<'tcx>,
 ) {
     use ExprKind::*;
-    let Expr { kind, ty: _, temp_lifetime: _, span: _ } = expr;
+    let Expr { kind, ty: _, temp_scope_id: _, span: _ } = expr;
     match *kind {
         Scope { value, region_scope: _, lint_level: _ } => {
             visitor.visit_expr(&visitor.thir()[value])

compiler/rustc_middle/src/thir/visit.rs

@@ -97,7 +97,6 @@ pub use self::region::{
     BoundRegion, BoundRegionKind, EarlyParamRegion, LateParamRegion, LateParamRegionKind, Region,
     RegionKind, RegionVid,
 };
-pub use self::rvalue_scopes::RvalueScopes;
 pub use self::sty::{
     AliasTy, Article, Binder, BoundTy, BoundTyKind, BoundVariableKind, CanonicalPolyFnSig,
     CoroutineArgsExt, EarlyBinder, FnSig, InlineConstArgs, InlineConstArgsParts, ParamConst,
@@ -156,7 +155,6 @@ mod list;
 mod opaque_types;
 mod predicate;
 mod region;
-mod rvalue_scopes;
 mod structural_impls;
 #[allow(hidden_glob_reexports)]
 mod sty;