Auto merge of #139781 - jhpratt:rollup-qadsjvb, r=jhpratt

Rollup of 9 pull requests

Successful merges:

 - #138336 (Improve `-Z crate-attr` diagnostics)
 - #139636 (Encode dep node edge count as u32 instead of usize)
 - #139666 (cleanup `mir_borrowck`)
 - #139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout)
 - #139699 (Proactively update coroutine drop shim's phase to account for later passes applied during shim query)
 - #139718 (enforce unsafe attributes in pre-2024 editions by default)
 - #139722 (Move some things to rustc_type_ir)
 - #139760 (UI tests: migrate remaining compile time `error-pattern`s to line annotations when possible)
 - #139776 (Switch attrs to `diagnostic::on_unimplemented`)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

Cargo.lock

@@ -719,6 +719,7 @@ version = "0.0.0"
 dependencies = [
  "anstyle-svg",
  "build_helper",
+ "camino",
  "colored",
  "diff",
  "getopts",
@@ -4671,6 +4672,7 @@ name = "rustdoc-gui-test"
 version = "0.1.0"
 dependencies = [
  "build_helper",
+ "camino",
  "compiletest",
  "getopts",
  "walkdir",

compiler/rustc_ast/src/attr/mod.rs

@@ -627,7 +627,7 @@ pub fn mk_doc_comment(
     Attribute { kind: AttrKind::DocComment(comment_kind, data), id: g.mk_attr_id(), style, span }
 }
 
-pub fn mk_attr(
+fn mk_attr(
     g: &AttrIdGenerator,
     style: AttrStyle,
     unsafety: Safety,

compiler/rustc_borrowck/src/lib.rs

@@ -21,6 +21,7 @@ use std::cell::RefCell;
 use std::marker::PhantomData;
 use std::ops::{ControlFlow, Deref};
 
+use borrow_set::LocalsStateAtExit;
 use root_cx::BorrowCheckRootCtxt;
 use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
@@ -303,33 +304,13 @@ fn do_mir_borrowck<'tcx>(
         root_cx.set_tainted_by_errors(e);
     }
 
-    let mut local_names = IndexVec::from_elem(None, &input_body.local_decls);
-    for var_debug_info in &input_body.var_debug_info {
-        if let VarDebugInfoContents::Place(place) = var_debug_info.value {
-            if let Some(local) = place.as_local() {
-                if let Some(prev_name) = local_names[local]
-                    && var_debug_info.name != prev_name
-                {
-                    span_bug!(
-                        var_debug_info.source_info.span,
-                        "local {:?} has many names (`{}` vs `{}`)",
-                        local,
-                        prev_name,
-                        var_debug_info.name
-                    );
-                }
-                local_names[local] = Some(var_debug_info.name);
-            }
-        }
-    }
-
     // Replace all regions with fresh inference variables. This
     // requires first making our own copy of the MIR. This copy will
     // be modified (in place) to contain non-lexical lifetimes. It
     // will have a lifetime tied to the inference context.
     let mut body_owned = input_body.clone();
     let mut promoted = input_promoted.to_owned();
-    let free_regions = nll::replace_regions_in_mir(&infcx, &mut body_owned, &mut promoted);
+    let universal_regions = nll::replace_regions_in_mir(&infcx, &mut body_owned, &mut promoted);
     let body = &body_owned; // no further changes
 
     let location_table = PoloniusLocationTable::new(body);
@@ -354,7 +335,7 @@ fn do_mir_borrowck<'tcx>(
     } = nll::compute_regions(
         root_cx,
         &infcx,
-        free_regions,
+        universal_regions,
         body,
         &promoted,
         &location_table,
@@ -367,24 +348,23 @@ fn do_mir_borrowck<'tcx>(
     // Dump MIR results into a file, if that is enabled. This lets us
     // write unit-tests, as well as helping with debugging.
     nll::dump_nll_mir(&infcx, body, &regioncx, &opt_closure_req, &borrow_set);
+    polonius::dump_polonius_mir(
+        &infcx,
+        body,
+        &regioncx,
+        &opt_closure_req,
+        &borrow_set,
+        polonius_diagnostics.as_ref(),
+    );
 
     // 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, 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
-};
+    nll::dump_annotation(&infcx, body, &regioncx, &opt_closure_req);
 
+    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 {
@@ -402,7 +382,6 @@ fn do_mir_borrowck<'tcx>(
             location_table: &location_table,
             movable_coroutine,
             fn_self_span_reported: Default::default(),
-            locals_are_invalidated_at_exit,
             access_place_error_reported: Default::default(),
             reservation_error_reported: Default::default(),
             uninitialized_error_reported: Default::default(),
@@ -434,14 +413,33 @@ fn do_mir_borrowck<'tcx>(
         promoted_mbcx.report_move_errors();
     }
 
+    let mut local_names = IndexVec::from_elem(None, &body.local_decls);
+    for var_debug_info in &body.var_debug_info {
+        if let VarDebugInfoContents::Place(place) = var_debug_info.value {
+            if let Some(local) = place.as_local() {
+                if let Some(prev_name) = local_names[local]
+                    && var_debug_info.name != prev_name
+                {
+                    span_bug!(
+                        var_debug_info.source_info.span,
+                        "local {:?} has many names (`{}` vs `{}`)",
+                        local,
+                        prev_name,
+                        var_debug_info.name
+                    );
+                }
+                local_names[local] = Some(var_debug_info.name);
+            }
+        }
+    }
+
     let mut mbcx = MirBorrowckCtxt {
         root_cx,
         infcx: &infcx,
         body,
         move_data: &move_data,
         location_table: &location_table,
         movable_coroutine,
-        locals_are_invalidated_at_exit,
         fn_self_span_reported: Default::default(),
         access_place_error_reported: Default::default(),
         reservation_error_reported: Default::default(),
@@ -454,9 +452,9 @@ fn do_mir_borrowck<'tcx>(
         local_names,
         region_names: RefCell::default(),
         next_region_name: RefCell::new(1),
-        polonius_output,
         move_errors: Vec::new(),
         diags_buffer,
+        polonius_output: polonius_output.as_deref(),
         polonius_diagnostics: polonius_diagnostics.as_ref(),
     };
 
@@ -473,16 +471,6 @@ fn do_mir_borrowck<'tcx>(
 
     mbcx.report_move_errors();
 
-    // If requested, dump polonius MIR.
-    polonius::dump_polonius_mir(
-        &infcx,
-        body,
-        &regioncx,
-        &borrow_set,
-        polonius_diagnostics.as_ref(),
-        &opt_closure_req,
-    );
-
     // For each non-user used mutable variable, check if it's been assigned from
     // a user-declared local. If so, then put that local into the used_mut set.
     // Note that this set is expected to be small - only upvars from closures
@@ -513,15 +501,14 @@ fn do_mir_borrowck<'tcx>(
     };
 
     let body_with_facts = if consumer_options.is_some() {
-        let output_facts = mbcx.polonius_output;
         Some(Box::new(BodyWithBorrowckFacts {
             body: body_owned,
             promoted,
             borrow_set,
             region_inference_context: regioncx,
             location_table: polonius_input.as_ref().map(|_| location_table),
             input_facts: polonius_input,
-            output_facts,
+            output_facts: polonius_output,
         }))
     } else {
         None
@@ -654,13 +641,6 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
     location_table: &'a PoloniusLocationTable,
 
     movable_coroutine: bool,
-    /// This keeps track of whether local variables are free-ed when the function
-    /// exits even without a `StorageDead`, which appears to be the case for
-    /// constants.
-    ///
-    /// I'm not sure this is the right approach - @eddyb could you try and
-    /// figure this out?
-    locals_are_invalidated_at_exit: bool,
     /// This field keeps track of when borrow errors are reported in the access_place function
     /// so that there is no duplicate reporting. This field cannot also be used for the conflicting
     /// borrow errors that is handled by the `reservation_error_reported` field as the inclusion
@@ -708,12 +688,11 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
     /// The counter for generating new region names.
     next_region_name: RefCell<usize>,
 
-    /// Results of Polonius analysis.
-    polonius_output: Option<Box<PoloniusOutput>>,
-
     diags_buffer: &'a mut BorrowckDiagnosticsBuffer<'infcx, 'tcx>,
     move_errors: Vec<MoveError<'tcx>>,
 
+    /// Results of Polonius analysis.
+    polonius_output: Option<&'a PoloniusOutput>,
     /// When using `-Zpolonius=next`: the data used to compute errors and diagnostics.
     polonius_diagnostics: Option<&'a PoloniusDiagnosticsContext>,
 }
@@ -937,13 +916,20 @@ impl<'a, 'tcx> ResultsVisitor<'a, 'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<
             | TerminatorKind::Return
             | TerminatorKind::TailCall { .. }
             | TerminatorKind::CoroutineDrop => {
-                // Returning from the function implicitly kills storage for all locals and statics.
-                // Often, the storage will already have been killed by an explicit
-                // StorageDead, but we don't always emit those (notably on unwind paths),
-                // so this "extra check" serves as a kind of backup.
-                for i in state.borrows.iter() {
-                    let borrow = &self.borrow_set[i];
-                    self.check_for_invalidation_at_exit(loc, borrow, span);
+                match self.borrow_set.locals_state_at_exit() {
+                    LocalsStateAtExit::AllAreInvalidated => {
+                        // Returning from the function implicitly kills storage for all locals and statics.
+                        // Often, the storage will already have been killed by an explicit
+                        // StorageDead, but we don't always emit those (notably on unwind paths),
+                        // so this "extra check" serves as a kind of backup.
+                        for i in state.borrows.iter() {
+                            let borrow = &self.borrow_set[i];
+                            self.check_for_invalidation_at_exit(loc, borrow, span);
+                        }
+                    }
+                    // If we do not implicitly invalidate all locals on exit,
+                    // we check for conflicts when dropping or moving this local.
+                    LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved: _ } => {}
                 }
             }
 
@@ -1715,22 +1701,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
         // we'll have a memory leak) and assume that all statics have a destructor.
         //
         // FIXME: allow thread-locals to borrow other thread locals?
-
-        let (might_be_alive, will_be_dropped) =
-            if self.body.local_decls[root_place.local].is_ref_to_thread_local() {
-                // Thread-locals might be dropped after the function exits
-                // We have to dereference the outer reference because
-                // borrows don't conflict behind shared references.
-                root_place.projection = TyCtxtConsts::DEREF_PROJECTION;
-                (true, true)
-            } else {
-                (false, self.locals_are_invalidated_at_exit)
-            };
-
-        if !will_be_dropped {
-            debug!("place_is_invalidated_at_exit({:?}) - won't be dropped", place);
-            return;
-        }
+        let might_be_alive = if self.body.local_decls[root_place.local].is_ref_to_thread_local() {
+            // Thread-locals might be dropped after the function exits
+            // We have to dereference the outer reference because
+            // borrows don't conflict behind shared references.
+            root_place.projection = TyCtxtConsts::DEREF_PROJECTION;
+            true
+        } else {
+            false
+        };
 
         let sd = if might_be_alive { Deep } else { Shallow(None) };
 

compiler/rustc_borrowck/src/nll.rs

@@ -21,7 +21,7 @@ use tracing::{debug, instrument};
 
 use crate::borrow_set::BorrowSet;
 use crate::consumers::ConsumerOptions;
-use crate::diagnostics::{BorrowckDiagnosticsBuffer, RegionErrors};
+use crate::diagnostics::RegionErrors;
 use crate::polonius::PoloniusDiagnosticsContext;
 use crate::polonius::legacy::{
     PoloniusFacts, PoloniusFactsExt, PoloniusLocationTable, PoloniusOutput,
@@ -117,11 +117,6 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         Rc::clone(&location_map),
     );
 
-    // Create the region inference context, taking ownership of the
-    // region inference data that was contained in `infcx`, and the
-    // base constraints generated by the type-check.
-    let var_infos = infcx.get_region_var_infos();
-
     // If requested, emit legacy polonius facts.
     polonius::legacy::emit_facts(
         &mut polonius_facts,
@@ -134,13 +129,8 @@ pub(crate) fn compute_regions<'a, 'tcx>(
         &constraints,
     );
 
-    let mut regioncx = RegionInferenceContext::new(
-        infcx,
-        var_infos,
-        constraints,
-        universal_region_relations,
-        location_map,
-    );
+    let mut regioncx =
+        RegionInferenceContext::new(infcx, constraints, universal_region_relations, location_map);
 
     // If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives constraints
     // and use them to compute loan liveness.
@@ -297,7 +287,6 @@ pub(super) fn dump_annotation<'tcx, 'infcx>(
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
     closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
-    diagnostics_buffer: &mut BorrowckDiagnosticsBuffer<'infcx, 'tcx>,
 ) {
     let tcx = infcx.tcx;
     let base_def_id = tcx.typeck_root_def_id(body.source.def_id());
@@ -335,13 +324,11 @@ pub(super) fn dump_annotation<'tcx, 'infcx>(
     } else {
         let mut err = infcx.dcx().struct_span_note(def_span, "no external requirements");
         regioncx.annotate(tcx, &mut err);
-
         err
     };
 
     // FIXME(@lcnr): We currently don't dump the inferred hidden types here.
-
-    diagnostics_buffer.buffer_non_error(err);
+    err.emit();
 }
 
 fn for_each_region_constraint<'tcx>(

compiler/rustc_borrowck/src/polonius/dump.rs

@@ -24,9 +24,9 @@ pub(crate) fn dump_polonius_mir<'tcx>(
     infcx: &BorrowckInferCtxt<'tcx>,
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
+    closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
     borrow_set: &BorrowSet<'tcx>,
     polonius_diagnostics: Option<&PoloniusDiagnosticsContext>,
-    closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
 ) {
     let tcx = infcx.tcx;
     if !tcx.sess.opts.unstable_opts.polonius.is_next_enabled() {