Sort all borrowck errors within a single typeck owner together

Author: oli-obk

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -34,13 +34,13 @@ use tracing::debug;
 
 use super::MirBorrowckCtxt;
 use super::borrow_set::BorrowData;
+use crate::LocalMutationIsAllowed;
 use crate::constraints::OutlivesConstraint;
 use crate::nll::ConstraintDescription;
 use crate::session_diagnostics::{
     CaptureArgLabel, CaptureReasonLabel, CaptureReasonNote, CaptureReasonSuggest, CaptureVarCause,
     CaptureVarKind, CaptureVarPathUseCause, OnClosureNote,
 };
-use crate::{BorrowCheckRootCtxt, LocalMutationIsAllowed};
 
 mod find_all_local_uses;
 mod find_use;
@@ -120,7 +120,7 @@ impl<'diag, 'tcx> BorrowckDiagnosticsBuffer<'diag, 'tcx> {
         self.buffered_diags.push(BufferedDiag::Error(diag));
     }
 
-    pub(crate) fn emit_errors(&mut self, root_cx: &mut BorrowCheckRootCtxt<'diag, 'tcx>) {
+    pub(crate) fn emit_errors(&mut self) {
         // Buffer any move errors that we collected and de-duplicated.
         for (_, (_, diag)) in std::mem::take(&mut self.buffered_move_errors) {
             // We have already set tainted for this error, so just buffer it.
@@ -136,7 +136,12 @@ impl<'diag, 'tcx> BorrowckDiagnosticsBuffer<'diag, 'tcx> {
         if !self.buffered_diags.is_empty() {
             self.buffered_diags.sort_by_key(|buffered_diag| buffered_diag.sort_span());
             for buffered_diag in self.buffered_diags.drain(..) {
-                root_cx.buffer_error(buffered_diag);
+                match buffered_diag {
+                    BufferedDiag::Error(diag) => {
+                        diag.emit();
+                    }
+                    BufferedDiag::NonError(diag) => diag.emit(),
+                }
             }
         }
     }

compiler/rustc_borrowck/src/lib.rs

@@ -394,8 +394,9 @@ fn borrowck_collect_region_constraints<'tcx>(
 /// Using the region constraints computed by [borrowck_collect_region_constraints]
 /// and the additional constraints from [BorrowCheckRootCtxt::handle_opaque_type_uses],
 /// compute the region graph and actually check for any borrowck errors.
-fn borrowck_check_region_constraints<'tcx>(
-    root_cx: &mut BorrowCheckRootCtxt<'_, 'tcx>,
+fn borrowck_check_region_constraints<'diag, 'tcx>(
+    root_cx: &mut BorrowCheckRootCtxt<'diag, 'tcx>,
+    diags_buffer: &mut BorrowckDiagnosticsBuffer<'diag, 'tcx>,
     CollectRegionConstraintsResult {
         infcx,
         body_owned,
@@ -462,7 +463,6 @@ fn borrowck_check_region_constraints<'tcx>(
     let movable_coroutine = body.coroutine.is_some()
         && tcx.coroutine_movability(def.to_def_id()) == hir::Movability::Movable;
 
-    let diags_buffer = &mut BorrowckDiagnosticsBuffer::default();
     // 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]); }`.
     for promoted_body in &promoted {
@@ -579,8 +579,6 @@ fn borrowck_check_region_constraints<'tcx>(
         used_mut_upvars: mbcx.used_mut_upvars,
     };
 
-    diags_buffer.emit_errors(root_cx);
-
     if let Some(guar) = infcx.tainted_by_errors() {
         root_cx.set_tainted_by_errors(guar);
     }

compiler/rustc_borrowck/src/root_cx.rs

@@ -12,7 +12,7 @@ use rustc_span::ErrorGuaranteed;
 use smallvec::SmallVec;
 
 use crate::consumers::BorrowckConsumer;
-use crate::diagnostics::BufferedDiag;
+use crate::diagnostics::BorrowckDiagnosticsBuffer;
 use crate::nll::compute_closure_requirements_modulo_opaques;
 use crate::region_infer::opaque_types::{
     UnexpectedHiddenRegion, apply_definition_site_hidden_types, clone_and_resolve_opaque_types,
@@ -46,7 +46,6 @@ pub(super) struct BorrowCheckRootCtxt<'diag, 'tcx: 'diag> {
     /// This should be `None` during normal compilation. See [`crate::consumers`] for more
     /// information on how this is used.
     pub consumer: Option<BorrowckConsumer<'tcx>>,
-    errors: Vec<BufferedDiag<'diag>>,
 }
 
 impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
@@ -65,7 +64,6 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
             propagated_borrowck_results: Default::default(),
             tainted_by_errors,
             consumer,
-            errors: Default::default(),
         }
     }
 
@@ -89,10 +87,9 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
     }
 
     pub(super) fn finalize(
-        mut self,
+        self,
     ) -> Result<&'tcx FxIndexMap<LocalDefId, ty::DefinitionSiteHiddenType<'tcx>>, ErrorGuaranteed>
     {
-        self.emit_errors();
         if let Some(guar) = self.tainted_by_errors.get() {
             Err(guar)
         } else {
@@ -168,7 +165,10 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
     /// which don't depend on opaque types. In this case they get removed from
     /// `collect_region_constraints_results` and the final result gets put into
     /// `propagated_borrowck_results`.
-    fn apply_closure_requirements_modulo_opaques(&mut self) {
+    fn apply_closure_requirements_modulo_opaques(
+        &mut self,
+        diags_buffer: &mut BorrowckDiagnosticsBuffer<'diag, 'tcx>,
+    ) {
         let mut closure_requirements_modulo_opaques = FxHashMap::default();
         // We need to `mem::take` both `self.collect_region_constraints_results` and
         // `input.deferred_closure_requirements` as we otherwise can't iterate over
@@ -226,7 +226,7 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
                 self.collect_region_constraints_results.insert(def_id, input);
             } else {
                 assert!(input.deferred_closure_requirements.is_empty());
-                let result = borrowck_check_region_constraints(self, input);
+                let result = borrowck_check_region_constraints(self, diags_buffer, input);
                 self.propagated_borrowck_results.insert(def_id, result);
             }
         }
@@ -281,14 +281,16 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
             self.collect_region_constraints_results.insert(def_id, result);
         }
 
+        let diags_buffer = &mut BorrowckDiagnosticsBuffer::default();
+
         // We now apply the closure requirements of nested bodies modulo
         // opaques. In case a body does not depend on opaque types, we
         // eagerly check its region constraints and use the final closure
         // requirements.
         //
         // We eagerly finish borrowck for bodies which don't depend on
         // opaques.
-        self.apply_closure_requirements_modulo_opaques();
+        self.apply_closure_requirements_modulo_opaques(diags_buffer);
 
         // We handle opaque type uses for all bodies together.
         self.handle_opaque_type_uses();
@@ -314,32 +316,9 @@ impl<'diag, 'tcx> BorrowCheckRootCtxt<'diag, 'tcx> {
                 );
             }
 
-            let result = borrowck_check_region_constraints(self, input);
+            let result = borrowck_check_region_constraints(self, diags_buffer, input);
             self.propagated_borrowck_results.insert(def_id, result);
         }
-    }
-
-    pub(crate) fn buffer_error(&mut self, buffered_diag: BufferedDiag<'_>) {
-        match buffered_diag {
-            BufferedDiag::Error(diag) => {
-                let diag = diag.with_dcx(self.dcx());
-                self.errors.push(BufferedDiag::Error(diag));
-            }
-            BufferedDiag::NonError(diag) => {
-                let diag = diag.with_dcx(self.dcx());
-                self.errors.push(BufferedDiag::NonError(diag));
-            }
-        }
-    }
-
-    pub(crate) fn emit_errors(&mut self) {
-        for buffered_diag in self.errors.drain(..) {
-            match buffered_diag {
-                BufferedDiag::Error(diag) => {
-                    diag.emit();
-                }
-                BufferedDiag::NonError(diag) => diag.emit(),
-            }
-        }
+        diags_buffer.emit_errors();
     }
 }

tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr

@@ -10,18 +10,23 @@ LL |     *x += 1;
 LL |     y
    |     - returning this value requires that `*x` is borrowed for `'1`
 
-error[E0506]: cannot assign to `*x` because it is borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:17:9
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:13:5
    |
-LL |     (async move || {
-   |                  - return type of async closure is &'1 i32
-...
-LL |         let y = &*x;
-   |                 --- `*x` is borrowed here
-LL |         *x += 1;
-   |         ^^^^^^^ `*x` is assigned to here but it was already borrowed
-LL |         y
-   |         - returning this value requires that `*x` is borrowed for `'1`
+LL |    pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
+   |                            - let's call the lifetime of this reference `'1`
+LL | //     (async move || {
+LL | ||
+LL | ||
+LL | ||         let y = &*x;
+LL | ||         *x += 1;
+LL | ||         y
+LL | ||     })()
+   | ||______^_- argument requires that borrow lasts for `'1`
+   | |_______|
+   |         creates a temporary value which is freed while still in use
+LL |    }
+   |    - temporary value is freed at the end of this statement
 
 error: lifetime may not live long enough
   --> $DIR/issue-74072-lifetime-name-annotations.rs:13:20
@@ -41,12 +46,25 @@ LL | |     })()
    |
    = note: closure implements `AsyncFnMut`, so references to captured variables can't escape the closure
 
+error[E0506]: cannot assign to `*x` because it is borrowed
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:17:9
+   |
+LL |     (async move || {
+   |                  - return type of async closure is &'1 i32
+...
+LL |         let y = &*x;
+   |                 --- `*x` is borrowed here
+LL |         *x += 1;
+   |         ^^^^^^^ `*x` is assigned to here but it was already borrowed
+LL |         y
+   |         - returning this value requires that `*x` is borrowed for `'1`
+
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:13:5
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:23:5
    |
-LL |    pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
-   |                            - let's call the lifetime of this reference `'1`
-LL | //     (async move || {
+LL |    pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
+   |                                                 - let's call the lifetime of this reference `'1`
+LL | //     (async move || -> &i32 {
 LL | ||
 LL | ||
 LL | ||         let y = &*x;
@@ -59,19 +77,6 @@ LL | ||     })()
 LL |    }
    |    - temporary value is freed at the end of this statement
 
-error[E0506]: cannot assign to `*x` because it is borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:27:9
-   |
-LL |     (async move || -> &i32 {
-   |                          - return type of async closure is &'1 i32
-...
-LL |         let y = &*x;
-   |                 --- `*x` is borrowed here
-LL |         *x += 1;
-   |         ^^^^^^^ `*x` is assigned to here but it was already borrowed
-LL |         y
-   |         - returning this value requires that `*x` is borrowed for `'1`
-
 error: lifetime may not live long enough
   --> $DIR/issue-74072-lifetime-name-annotations.rs:23:28
    |
@@ -90,23 +95,18 @@ LL | |     })()
    |
    = note: closure implements `AsyncFnMut`, so references to captured variables can't escape the closure
 
-error[E0716]: temporary value dropped while borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:23:5
+error[E0506]: cannot assign to `*x` because it is borrowed
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:27:9
    |
-LL |    pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
-   |                                                 - let's call the lifetime of this reference `'1`
-LL | //     (async move || -> &i32 {
-LL | ||
-LL | ||
-LL | ||         let y = &*x;
-LL | ||         *x += 1;
-LL | ||         y
-LL | ||     })()
-   | ||______^_- argument requires that borrow lasts for `'1`
-   | |_______|
-   |         creates a temporary value which is freed while still in use
-LL |    }
-   |    - temporary value is freed at the end of this statement
+LL |     (async move || -> &i32 {
+   |                          - return type of async closure is &'1 i32
+...
+LL |         let y = &*x;
+   |                 --- `*x` is borrowed here
+LL |         *x += 1;
+   |         ^^^^^^^ `*x` is assigned to here but it was already borrowed
+LL |         y
+   |         - returning this value requires that `*x` is borrowed for `'1`
 
 error[E0506]: cannot assign to `*x` because it is borrowed
   --> $DIR/issue-74072-lifetime-name-annotations.rs:35:9

tests/ui/async-await/issues/issue-62097.stderr

@@ -1,3 +1,17 @@
+error[E0521]: borrowed data escapes outside of method
+  --> $DIR/issue-62097.rs:13:9
+   |
+LL |     pub async fn run_dummy_fn(&self) {
+   |                               -----
+   |                               |
+   |                               `self` is a reference that is only valid in the method body
+   |                               let's call the lifetime of this reference `'1`
+LL |         foo(|| self.bar()).await;
+   |         ^^^^^^^^^^^^^^^^^^
+   |         |
+   |         `self` escapes the method body here
+   |         argument requires that `'1` must outlive `'static`
+
 error[E0373]: closure may outlive the current function, but it borrows `self`, which is owned by the current function
   --> $DIR/issue-62097.rs:13:13
    |
@@ -16,20 +30,6 @@ help: to force the closure to take ownership of `self` (and any other referenced
 LL |         foo(move || self.bar()).await;
    |             ++++
 
-error[E0521]: borrowed data escapes outside of method
-  --> $DIR/issue-62097.rs:13:9
-   |
-LL |     pub async fn run_dummy_fn(&self) {
-   |                               -----
-   |                               |
-   |                               `self` is a reference that is only valid in the method body
-   |                               let's call the lifetime of this reference `'1`
-LL |         foo(|| self.bar()).await;
-   |         ^^^^^^^^^^^^^^^^^^
-   |         |
-   |         `self` escapes the method body here
-   |         argument requires that `'1` must outlive `'static`
-
 error: aborting due to 2 previous errors
 
 Some errors have detailed explanations: E0373, E0521.

tests/ui/borrowck/borrowck-closures-mut-of-imm.stderr

@@ -4,12 +4,6 @@ error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference
 LL |     let mut c1 = || set(&mut *x);
    |                         ^^^^^^^ cannot borrow as mutable
 
-error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference
-  --> $DIR/borrowck-closures-mut-of-imm.rs:12:25
-   |
-LL |     let mut c2 = || set(&mut *x);
-   |                         ^^^^^^^ cannot borrow as mutable
-
 error[E0524]: two closures require unique access to `x` at the same time
   --> $DIR/borrowck-closures-mut-of-imm.rs:12:18
    |
@@ -26,6 +20,12 @@ LL |     let mut c2 = || set(&mut *x);
 LL |     c2(); c1();
    |           -- first borrow later used here
 
+error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference
+  --> $DIR/borrowck-closures-mut-of-imm.rs:12:25
+   |
+LL |     let mut c2 = || set(&mut *x);
+   |                         ^^^^^^^ cannot borrow as mutable
+
 error: aborting due to 3 previous errors
 
 Some errors have detailed explanations: E0524, E0596.