Auto merge of #138499 - lcnr:borrowck-typeck_root, r=<try>

borrowck typeck children together with their root

This introduces new cycle errors, even with `feature(inline_const_pat)` removed, see the `non-structural-match-types-cycle-err.rs` test.

The new cycle error happens as the layout of `async`-blocks relies on their `optimized_mir`. As that now depends on `mir_borrowck` of its typeck parent, computing the layout of an `async`-block during MIR building, e.g. when evaluating a named `const` pattern. I think there's currently no way to have a named const pattern whose type references an async block while being allowed? cc `@oli-obk` `@RalfJung`

I cannot think of other cases where we currently rely on the MIR of a typeck children while borrowchecking their parent. The crater run came back without any breakage. My work here will prevent any future features which rely on this as we'll get locked into borrowchecking them together as I continue to work on rust-lang/types-team#129, cc `@rust-lang/types.`

r? compiler-errors

Author: bors

compiler/rustc_borrowck/src/consumers.rs

@@ -15,6 +15,7 @@ pub use super::polonius::legacy::{
     RichLocation, RustcFacts,
 };
 pub use super::region_infer::RegionInferenceContext;
+use crate::{BorrowCheckRootCtxt, do_mir_borrowck};
 
 /// Options determining the output behavior of [`get_body_with_borrowck_facts`].
 ///
@@ -97,8 +98,9 @@ pub struct BodyWithBorrowckFacts<'tcx> {
 /// *   Polonius is highly unstable, so expect regular changes in its signature or other details.
 pub fn get_body_with_borrowck_facts(
     tcx: TyCtxt<'_>,
-    def: LocalDefId,
+    def_id: LocalDefId,
     options: ConsumerOptions,
 ) -> BodyWithBorrowckFacts<'_> {
-    *super::do_mir_borrowck(tcx, def, Some(options)).1.unwrap()
+    let mut root_cx = BorrowCheckRootCtxt::new(tcx, def_id);
+    *do_mir_borrowck(&mut root_cx, def_id, Some(options)).1.unwrap()
 }

compiler/rustc_borrowck/src/lib.rs

@@ -21,6 +21,7 @@ use std::cell::RefCell;
 use std::marker::PhantomData;
 use std::ops::{ControlFlow, Deref};
 
+use root_cx::BorrowCheckRootCtxt;
 use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
 use rustc_data_structures::graph::dominators::Dominators;
@@ -35,7 +36,7 @@ use rustc_infer::infer::{
 };
 use rustc_middle::mir::*;
 use rustc_middle::query::Providers;
-use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt, TypingMode, fold_regions};
+use rustc_middle::ty::{self, ParamEnv, RegionVid, Ty, TyCtxt, TypingMode, fold_regions};
 use rustc_middle::{bug, span_bug};
 use rustc_mir_dataflow::impls::{
     EverInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces,
@@ -45,7 +46,7 @@ use rustc_mir_dataflow::move_paths::{
 };
 use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
 use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
-use rustc_span::{Span, Symbol};
+use rustc_span::{ErrorGuaranteed, Span, Symbol};
 use smallvec::SmallVec;
 use tracing::{debug, instrument};
 
@@ -73,14 +74,14 @@ mod def_use;
 mod diagnostics;
 mod member_constraints;
 mod nll;
-mod opaque_types;
 mod path_utils;
 mod place_ext;
 mod places_conflict;
 mod polonius;
 mod prefixes;
 mod region_infer;
 mod renumber;
+mod root_cx;
 mod session_diagnostics;
 mod type_check;
 mod universal_regions;
@@ -102,44 +103,194 @@ pub fn provide(providers: &mut Providers) {
     *providers = Providers { mir_borrowck, ..*providers };
 }
 
-fn mir_borrowck(tcx: TyCtxt<'_>, def: LocalDefId) -> &BorrowCheckResult<'_> {
+fn mir_borrowck(
+    tcx: TyCtxt<'_>,
+    def: LocalDefId,
+) -> Result<&ConcreteOpaqueTypes<'_>, ErrorGuaranteed> {
+    assert!(!tcx.is_typeck_child(def.to_def_id()));
     let (input_body, _) = tcx.mir_promoted(def);
+    debug!("run query mir_borrowck: {}", tcx.def_path_str(def));
+
     let input_body: &Body<'_> = &input_body.borrow();
-    if input_body.should_skip() || input_body.tainted_by_errors.is_some() {
-        debug!("Skipping borrowck because of injected body or tainted body");
-        // Let's make up a borrowck result! Fun times!
-        let result = BorrowCheckResult {
-            concrete_opaque_types: FxIndexMap::default(),
-            closure_requirements: None,
-            used_mut_upvars: SmallVec::new(),
-            tainted_by_errors: input_body.tainted_by_errors,
-        };
-        return tcx.arena.alloc(result);
+    if let Some(guar) = input_body.tainted_by_errors {
+        debug!("Skipping borrowck because of tainted body");
+        Err(guar)
+    } else if input_body.should_skip() {
+        debug!("Skipping borrowck because of injected body");
+        let opaque_types = ConcreteOpaqueTypes(Default::default());
+        Ok(tcx.arena.alloc(opaque_types))
+    } else {
+        let mut root_cx = BorrowCheckRootCtxt::new(tcx, def);
+        let BorrowCheckResult { closure_requirements, used_mut_upvars } =
+            do_mir_borrowck(&mut root_cx, def, None).0;
+        debug_assert!(closure_requirements.is_none());
+        debug_assert!(used_mut_upvars.is_empty());
+        root_cx.finalize()
     }
+}
+
+#[derive(Debug)]
+struct BorrowCheckResult<'tcx> {
+    closure_requirements: Option<ClosureRegionRequirements<'tcx>>,
+    used_mut_upvars: SmallVec<[FieldIdx; 8]>,
+}
+
+/// After we borrow check a closure, we are left with various
+/// requirements that we have inferred between the free regions that
+/// appear in the closure's signature or on its field types. These
+/// requirements are then verified and proved by the closure's
+/// creating function. This struct encodes those requirements.
+///
+/// The requirements are listed as being between various `RegionVid`. The 0th
+/// region refers to `'static`; subsequent region vids refer to the free
+/// regions that appear in the closure (or coroutine's) type, in order of
+/// appearance. (This numbering is actually defined by the `UniversalRegions`
+/// struct in the NLL region checker. See for example
+/// `UniversalRegions::closure_mapping`.) Note the free regions in the
+/// closure's signature and captures are erased.
+///
+/// Example: If type check produces a closure with the closure args:
+///
+/// ```text
+/// ClosureArgs = [
+///     'a,                                         // From the parent.
+///     'b,
+///     i8,                                         // the "closure kind"
+///     for<'x> fn(&'<erased> &'x u32) -> &'x u32,  // the "closure signature"
+///     &'<erased> String,                          // some upvar
+/// ]
+/// ```
+///
+/// We would "renumber" each free region to a unique vid, as follows:
+///
+/// ```text
+/// ClosureArgs = [
+///     '1,                                         // From the parent.
+///     '2,
+///     i8,                                         // the "closure kind"
+///     for<'x> fn(&'3 &'x u32) -> &'x u32,         // the "closure signature"
+///     &'4 String,                                 // some upvar
+/// ]
+/// ```
+///
+/// Now the code might impose a requirement like `'1: '2`. When an
+/// instance of the closure is created, the corresponding free regions
+/// can be extracted from its type and constrained to have the given
+/// outlives relationship.
+#[derive(Clone, Debug)]
+pub struct ClosureRegionRequirements<'tcx> {
+    /// The number of external regions defined on the closure. In our
+    /// example above, it would be 3 -- one for `'static`, then `'1`
+    /// and `'2`. This is just used for a sanity check later on, to
+    /// make sure that the number of regions we see at the callsite
+    /// matches.
+    pub num_external_vids: usize,
+
+    /// Requirements between the various free regions defined in
+    /// indices.
+    pub outlives_requirements: Vec<ClosureOutlivesRequirement<'tcx>>,
+}
+
+/// Indicates an outlives-constraint between a type or between two
+/// free regions declared on the closure.
+#[derive(Copy, Clone, Debug)]
+pub struct ClosureOutlivesRequirement<'tcx> {
+    // This region or type ...
+    pub subject: ClosureOutlivesSubject<'tcx>,
 
-    let borrowck_result = do_mir_borrowck(tcx, def, None).0;
-    debug!("mir_borrowck done");
+    // ... must outlive this one.
+    pub outlived_free_region: ty::RegionVid,
 
-    tcx.arena.alloc(borrowck_result)
+    // If not, report an error here ...
+    pub blame_span: Span,
+
+    // ... due to this reason.
+    pub category: ConstraintCategory<'tcx>,
+}
+
+// Make sure this enum doesn't unintentionally grow
+#[cfg(target_pointer_width = "64")]
+rustc_data_structures::static_assert_size!(ConstraintCategory<'_>, 16);
+
+/// The subject of a `ClosureOutlivesRequirement` -- that is, the thing
+/// that must outlive some region.
+#[derive(Copy, Clone, Debug)]
+pub enum ClosureOutlivesSubject<'tcx> {
+    /// Subject is a type, typically a type parameter, but could also
+    /// be a projection. Indicates a requirement like `T: 'a` being
+    /// passed to the caller, where the type here is `T`.
+    Ty(ClosureOutlivesSubjectTy<'tcx>),
+
+    /// Subject is a free region from the closure. Indicates a requirement
+    /// like `'a: 'b` being passed to the caller; the region here is `'a`.
+    Region(ty::RegionVid),
+}
+
+/// Represents a `ty::Ty` for use in [`ClosureOutlivesSubject`].
+///
+/// This abstraction is necessary because the type may include `ReVar` regions,
+/// which is what we use internally within NLL code, and they can't be used in
+/// a query response.
+///
+/// DO NOT implement `TypeVisitable` or `TypeFoldable` traits, because this
+/// type is not recognized as a binder for late-bound region.
+#[derive(Copy, Clone, Debug)]
+pub struct ClosureOutlivesSubjectTy<'tcx> {
+    inner: Ty<'tcx>,
+}
+
+impl<'tcx> ClosureOutlivesSubjectTy<'tcx> {
+    /// All regions of `ty` must be of kind `ReVar` and must represent
+    /// universal regions *external* to the closure.
+    pub fn bind(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Self {
+        let inner = fold_regions(tcx, ty, |r, depth| match r.kind() {
+            ty::ReVar(vid) => {
+                let br = ty::BoundRegion {
+                    var: ty::BoundVar::from_usize(vid.index()),
+                    kind: ty::BoundRegionKind::Anon,
+                };
+                ty::Region::new_bound(tcx, depth, br)
+            }
+            _ => bug!("unexpected region in ClosureOutlivesSubjectTy: {r:?}"),
+        });
+
+        Self { inner }
+    }
+
+    pub fn instantiate(
+        self,
+        tcx: TyCtxt<'tcx>,
+        mut map: impl FnMut(ty::RegionVid) -> ty::Region<'tcx>,
+    ) -> Ty<'tcx> {
+        fold_regions(tcx, self.inner, |r, depth| match r.kind() {
+            ty::ReBound(debruijn, br) => {
+                debug_assert_eq!(debruijn, depth);
+                map(ty::RegionVid::from_usize(br.var.index()))
+            }
+            _ => bug!("unexpected region {r:?}"),
+        })
+    }
 }
 
 /// Perform the actual borrow checking.
 ///
 /// Use `consumer_options: None` for the default behavior of returning
 /// [`BorrowCheckResult`] only. Otherwise, return [`BodyWithBorrowckFacts`] according
 /// to the given [`ConsumerOptions`].
-#[instrument(skip(tcx), level = "debug")]
+#[instrument(skip(root_cx), level = "debug")]
 fn do_mir_borrowck<'tcx>(
-    tcx: TyCtxt<'tcx>,
+    root_cx: &mut BorrowCheckRootCtxt<'tcx>,
     def: LocalDefId,
     consumer_options: Option<ConsumerOptions>,
 ) -> (BorrowCheckResult<'tcx>, Option<Box<BodyWithBorrowckFacts<'tcx>>>) {
+    let tcx = root_cx.tcx;
     let infcx = BorrowckInferCtxt::new(tcx, def);
     let (input_body, promoted) = tcx.mir_promoted(def);
     let input_body: &Body<'_> = &input_body.borrow();
     let input_promoted: &IndexSlice<_, _> = &promoted.borrow();
     if let Some(e) = input_body.tainted_by_errors {
         infcx.set_tainted_by_errors(e);
+        root_cx.set_tainted_by_errors(e);
     }
 
     let mut local_names = IndexVec::from_elem(None, &input_body.local_decls);
@@ -191,13 +342,13 @@ fn do_mir_borrowck<'tcx>(
     // Compute non-lexical lifetimes.
     let nll::NllOutput {
         regioncx,
-        concrete_opaque_types,
         polonius_input,
         polonius_output,
         opt_closure_req,
         nll_errors,
         polonius_diagnostics,
     } = nll::compute_regions(
+        root_cx,
         &infcx,
         free_regions,
         body,
@@ -216,26 +367,19 @@ fn do_mir_borrowck<'tcx>(
     // We also have a `#[rustc_regions]` annotation that causes us to dump
     // information.
     let diags_buffer = &mut BorrowckDiagnosticsBuffer::default();
-    nll::dump_annotation(
-        &infcx,
-        body,
-        &regioncx,
-        &opt_closure_req,
-        &concrete_opaque_types,
-        diags_buffer,
-    );
+    nll::dump_annotation(&infcx, body, &regioncx, &opt_closure_req, diags_buffer);
 
     let movable_coroutine =
-        // The first argument is the coroutine type passed by value
-        if let Some(local) = body.local_decls.raw.get(1)
-        // Get the interior types and args which typeck computed
-        && let ty::Coroutine(def_id, _) = *local.ty.kind()
-        && tcx.coroutine_movability(def_id) == hir::Movability::Movable
-    {
-        true
-    } else {
-        false
-    };
+    // The first argument is the coroutine type passed by value
+    if let Some(local) = body.local_decls.raw.get(1)
+    // Get the interior types and args which typeck computed
+    && let ty::Coroutine(def_id, _) = *local.ty.kind()
+    && tcx.coroutine_movability(def_id) == hir::Movability::Movable
+{
+    true
+} else {
+    false
+};
 
     // While promoteds should mostly be correct by construction, we need to check them for
     // invalid moves to detect moving out of arrays:`struct S; fn main() { &([S][0]); }`.
@@ -246,6 +390,7 @@ fn do_mir_borrowck<'tcx>(
         // this check out of `MirBorrowckCtxt`, actually doing so is far from trivial.
         let move_data = MoveData::gather_moves(promoted_body, tcx, |_| true);
         let mut promoted_mbcx = MirBorrowckCtxt {
+            root_cx,
             infcx: &infcx,
             body: promoted_body,
             move_data: &move_data,
@@ -286,6 +431,7 @@ fn do_mir_borrowck<'tcx>(
     }
 
     let mut mbcx = MirBorrowckCtxt {
+        root_cx,
         infcx: &infcx,
         body,
         move_data: &move_data,
@@ -353,13 +499,13 @@ fn do_mir_borrowck<'tcx>(
 
     debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
     mbcx.lint_unused_mut();
-    let tainted_by_errors = mbcx.emit_errors();
+    if let Some(guar) = mbcx.emit_errors() {
+        mbcx.root_cx.set_tainted_by_errors(guar);
+    }
 
     let result = BorrowCheckResult {
-        concrete_opaque_types: concrete_opaque_types.into_inner(),
         closure_requirements: opt_closure_req,
         used_mut_upvars: mbcx.used_mut_upvars,
-        tainted_by_errors,
     };
 
     let body_with_facts = if consumer_options.is_some() {
@@ -512,6 +658,7 @@ impl<'tcx> Deref for BorrowckInferCtxt<'tcx> {
 }
 
 struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
+    root_cx: &'a mut BorrowCheckRootCtxt<'tcx>,
     infcx: &'infcx BorrowckInferCtxt<'tcx>,
     body: &'a Body<'tcx>,
     move_data: &'a MoveData<'tcx>,
@@ -1385,11 +1532,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
                     | AggregateKind::CoroutineClosure(def_id, _)
                     | AggregateKind::Coroutine(def_id, _) => {
                         let def_id = def_id.expect_local();
-                        let BorrowCheckResult { used_mut_upvars, .. } =
-                            self.infcx.tcx.mir_borrowck(def_id);
+                        let used_mut_upvars = self.root_cx.used_mut_upvars(def_id);
                         debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
-                        for field in used_mut_upvars {
-                            self.propagate_closure_used_mut_upvar(&operands[*field]);
+                        // FIXME: We're cloning the `SmallVec` here to avoid borrowing `root_cx`
+                        // when calling `propagate_closure_used_mut_upvar`. This should ideally
+                        // be unnecessary.
+                        for field in used_mut_upvars.clone() {
+                            self.propagate_closure_used_mut_upvar(&operands[field]);
                         }
                     }
                     AggregateKind::Adt(..)