Rollup merge of #66789 - eddyb:mir-source-scope-local-data, r=oli-obk

rustc: move mir::SourceScopeLocalData to a field of SourceScopeData.

By having one `ClearCrossCrate<SourceScopeLocalData>` for each scope, as opposed to a single `ClearCrossCrate` for all the `SourceScopeLocalData`s, we can represent the fact that some scopes have `SourceScopeLocalData` associated with them, and some don't.

This is useful when doing MIR inlining across crates, because the `ClearCrossCrate` will be `Clear` for the cross-crate MIR scopes and `Set` for the local ones.

Also see https://github.com/rust-lang/rust/pull/66203#issuecomment-555589574 for some context around this approach.

Fixes #51314.

src/librustc/mir/mod.rs

@@ -104,10 +104,6 @@ pub struct Body<'tcx> {
     /// and used for debuginfo. Indexed by a `SourceScope`.
     pub source_scopes: IndexVec<SourceScope, SourceScopeData>,
 
-    /// Crate-local information for each source scope, that can't (and
-    /// needn't) be tracked across crates.
-    pub source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
-
     /// The yield type of the function, if it is a generator.
     pub yield_ty: Option<Ty<'tcx>>,
 
@@ -167,7 +163,6 @@ impl<'tcx> Body<'tcx> {
     pub fn new(
         basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
         source_scopes: IndexVec<SourceScope, SourceScopeData>,
-        source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
         local_decls: LocalDecls<'tcx>,
         user_type_annotations: CanonicalUserTypeAnnotations<'tcx>,
         arg_count: usize,
@@ -188,7 +183,6 @@ impl<'tcx> Body<'tcx> {
             phase: MirPhase::Build,
             basic_blocks,
             source_scopes,
-            source_scope_local_data,
             yield_ty: None,
             generator_drop: None,
             generator_layout: None,
@@ -435,6 +429,13 @@ pub enum ClearCrossCrate<T> {
 }
 
 impl<T> ClearCrossCrate<T> {
+    pub fn as_ref(&'a self) -> ClearCrossCrate<&'a T> {
+        match self {
+            ClearCrossCrate::Clear => ClearCrossCrate::Clear,
+            ClearCrossCrate::Set(v) => ClearCrossCrate::Set(v),
+        }
+    }
+
     pub fn assert_crate_local(self) -> T {
         match self {
             ClearCrossCrate::Clear => bug!("unwrapping cross-crate data"),
@@ -2027,6 +2028,10 @@ rustc_index::newtype_index! {
 pub struct SourceScopeData {
     pub span: Span,
     pub parent_scope: Option<SourceScope>,
+
+    /// Crate-local information for this source scope, that can't (and
+    /// needn't) be tracked across crates.
+    pub local_data: ClearCrossCrate<SourceScopeLocalData>,
 }
 
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]

src/librustc/mir/visit.rs

@@ -317,6 +317,7 @@ macro_rules! make_mir_visitor {
                 let SourceScopeData {
                     span,
                     parent_scope,
+                    local_data: _,
                 } = scope_data;
 
                 self.visit_span(span);

src/librustc_mir/borrow_check/mod.rs

@@ -300,11 +300,10 @@ fn do_mir_borrowck<'a, 'tcx>(
         let mut initial_diag =
             mbcx.report_conflicting_borrow(location, (&place, span), bk, &borrow);
 
-        let lint_root = if let ClearCrossCrate::Set(ref vsi) = mbcx.body.source_scope_local_data {
+        let scope = mbcx.body.source_info(location).scope;
-            let scope = mbcx.body.source_info(location).scope;
+        let lint_root = match &mbcx.body.source_scopes[scope].local_data {
-            vsi[scope].lint_root
+            ClearCrossCrate::Set(data) => data.lint_root,
-        } else {
+            _ => id,
-            id
         };
 
         // Span and message don't matter; we overwrite them below anyway
@@ -338,38 +337,40 @@ fn do_mir_borrowck<'a, 'tcx>(
     debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
     let used_mut = mbcx.used_mut;
     for local in mbcx.body.mut_vars_and_args_iter().filter(|local| !used_mut.contains(local)) {
-        if let ClearCrossCrate::Set(ref vsi) = mbcx.body.source_scope_local_data {
+        let local_decl = &mbcx.body.local_decls[local];
-            let local_decl = &mbcx.body.local_decls[local];
+        let lint_root = match &mbcx.body.source_scopes[local_decl.source_info.scope].local_data {
+            ClearCrossCrate::Set(data) => data.lint_root,
+            _ => continue,
+        };
 
-            // Skip over locals that begin with an underscore or have no name
+        // Skip over locals that begin with an underscore or have no name
-            match mbcx.local_names[local] {
+        match mbcx.local_names[local] {
-                Some(name) => if name.as_str().starts_with("_") {
+            Some(name) => if name.as_str().starts_with("_") {
-                    continue;
-                },
-                None => continue,
-            }
-
-            let span = local_decl.source_info.span;
-            if span.desugaring_kind().is_some() {
-                // If the `mut` arises as part of a desugaring, we should ignore it.
                 continue;
-            }
+            },
-
+            None => continue,
-            let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);
-            tcx.struct_span_lint_hir(
-                UNUSED_MUT,
-                vsi[local_decl.source_info.scope].lint_root,
-                span,
-                "variable does not need to be mutable",
-            )
-            .span_suggestion_short(
-                mut_span,
-                "remove this `mut`",
-                String::new(),
-                Applicability::MachineApplicable,
-            )
-            .emit();
         }
+
+        let span = local_decl.source_info.span;
+        if span.desugaring_kind().is_some() {
+            // If the `mut` arises as part of a desugaring, we should ignore it.
+            continue;
+        }
+
+        let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);
+        tcx.struct_span_lint_hir(
+            UNUSED_MUT,
+            lint_root,
+            span,
+            "variable does not need to be mutable",
+        )
+        .span_suggestion_short(
+            mut_span,
+            "remove this `mut`",
+            String::new(),
+            Applicability::MachineApplicable,
+        )
+        .emit();
     }
 
     // Buffer any move errors that we collected and de-duplicated.

src/librustc_mir/build/mod.rs

@@ -309,7 +309,6 @@ struct Builder<'a, 'tcx> {
     /// The vector of all scopes that we have created thus far;
     /// we track this for debuginfo later.
     source_scopes: IndexVec<SourceScope, SourceScopeData>,
-    source_scope_local_data: IndexVec<SourceScope, SourceScopeLocalData>,
     source_scope: SourceScope,
 
     /// The guard-context: each time we build the guard expression for
@@ -704,7 +703,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             block_context: BlockContext::new(),
             source_scopes: IndexVec::new(),
             source_scope: OUTERMOST_SOURCE_SCOPE,
-            source_scope_local_data: IndexVec::new(),
             guard_context: vec![],
             push_unsafe_count: 0,
             unpushed_unsafe: safety,
@@ -741,7 +739,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         Body::new(
             self.cfg.basic_blocks,
             self.source_scopes,
-            ClearCrossCrate::Set(self.source_scope_local_data),
             self.local_decls,
             self.canonical_user_type_annotations,
             self.arg_count,
@@ -942,7 +939,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             self.hir.root_lint_level
         );
         let parent_root = tcx.maybe_lint_level_root_bounded(
-            self.source_scope_local_data[original_source_scope].lint_root,
+            self.source_scopes[original_source_scope]
+                .local_data
+                .as_ref()
+                .assert_crate_local()
+                .lint_root,
             self.hir.root_lint_level,
         );
         if current_root != parent_root {

src/librustc_mir/build/scope.rs

@@ -436,7 +436,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             // We estimate the true lint roots here to avoid creating a lot of source scopes.
 
             let parent_root = tcx.maybe_lint_level_root_bounded(
-                self.source_scope_local_data[source_scope].lint_root,
+                self.source_scopes[source_scope]
+                    .local_data
+                    .as_ref()
+                    .assert_crate_local()
+                    .lint_root,
                 self.hir.root_lint_level,
             );
             let current_root = tcx.maybe_lint_level_root_bounded(
@@ -654,23 +658,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let parent = self.source_scope;
         debug!("new_source_scope({:?}, {:?}, {:?}) - parent({:?})={:?}",
                span, lint_level, safety,
-               parent, self.source_scope_local_data.get(parent));
+               parent, self.source_scopes.get(parent));
-        let scope = self.source_scopes.push(SourceScopeData {
-            span,
-            parent_scope: Some(parent),
-        });
         let scope_local_data = SourceScopeLocalData {
             lint_root: if let LintLevel::Explicit(lint_root) = lint_level {
                 lint_root
             } else {
-                self.source_scope_local_data[parent].lint_root
+                self.source_scopes[parent].local_data.as_ref().assert_crate_local().lint_root
             },
             safety: safety.unwrap_or_else(|| {
-                self.source_scope_local_data[parent].safety
+                self.source_scopes[parent].local_data.as_ref().assert_crate_local().safety
             })
         };
-        self.source_scope_local_data.push(scope_local_data);
+        self.source_scopes.push(SourceScopeData {
-        scope
+            span,
+            parent_scope: Some(parent),
+            local_data: ClearCrossCrate::Set(scope_local_data),
+        })
     }
 
     /// Given a span and the current source scope, make a SourceInfo.

src/librustc_mir/interpret/eval_context.rs

@@ -849,8 +849,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 } else {
                     block.terminator().source_info
                 };
-                match body.source_scope_local_data {
+                match &body.source_scopes[source_info.scope].local_data {
-                    mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
+                    mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
                     mir::ClearCrossCrate::Clear => None,
                 }
             });

src/librustc_mir/shim.rs

@@ -198,9 +198,6 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>)
 
     let mut body = new_body(
         blocks,
-        IndexVec::from_elem_n(
-            SourceScopeData { span, parent_scope: None }, 1
-        ),
         local_decls_for_sig(&sig, span),
         sig.inputs().len(),
         span);
@@ -244,15 +241,16 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>)
 
 fn new_body<'tcx>(
     basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
-    source_scopes: IndexVec<SourceScope, SourceScopeData>,
     local_decls: IndexVec<Local, LocalDecl<'tcx>>,
     arg_count: usize,
     span: Span,
 ) -> Body<'tcx> {
     Body::new(
         basic_blocks,
-        source_scopes,
+        IndexVec::from_elem_n(
-        ClearCrossCrate::Clear,
+            SourceScopeData { span, parent_scope: None, local_data: ClearCrossCrate::Clear },
+            1,
+        ),
         local_decls,
         IndexVec::new(),
         arg_count,
@@ -380,9 +378,6 @@ impl CloneShimBuilder<'tcx> {
     fn into_mir(self) -> Body<'tcx> {
         new_body(
             self.blocks,
-            IndexVec::from_elem_n(
-                SourceScopeData { span: self.span, parent_scope: None }, 1
-            ),
             self.local_decls,
             self.sig.inputs().len(),
             self.span,
@@ -836,9 +831,6 @@ fn build_call_shim<'tcx>(
 
     let mut body = new_body(
         blocks,
-        IndexVec::from_elem_n(
-            SourceScopeData { span, parent_scope: None }, 1
-        ),
         local_decls,
         sig.inputs().len(),
         span,
@@ -919,9 +911,6 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> &Body<'_> {
 
     let body = new_body(
         IndexVec::from_elem_n(start_block, 1),
-        IndexVec::from_elem_n(
-            SourceScopeData { span, parent_scope: None }, 1
-        ),
         local_decls,
         sig.inputs().len(),
         span,

src/librustc_mir/transform/check_unsafety.rs

@@ -1,6 +1,4 @@
 use rustc_data_structures::fx::FxHashSet;
-use rustc_index::vec::IndexVec;
-use rustc_data_structures::sync::Lrc;
 
 use rustc::ty::query::Providers;
 use rustc::ty::{self, TyCtxt};
@@ -24,7 +22,6 @@ pub struct UnsafetyChecker<'a, 'tcx> {
     body: &'a Body<'tcx>,
     const_context: bool,
     min_const_fn: bool,
-    source_scope_local_data: &'a IndexVec<SourceScope, SourceScopeLocalData>,
     violations: Vec<UnsafetyViolation>,
     source_info: SourceInfo,
     tcx: TyCtxt<'tcx>,
@@ -39,7 +36,6 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
         const_context: bool,
         min_const_fn: bool,
         body: &'a Body<'tcx>,
-        source_scope_local_data: &'a IndexVec<SourceScope, SourceScopeLocalData>,
         tcx: TyCtxt<'tcx>,
         param_env: ty::ParamEnv<'tcx>,
     ) -> Self {
@@ -51,7 +47,6 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
             body,
             const_context,
             min_const_fn,
-            source_scope_local_data,
             violations: vec![],
             source_info: SourceInfo {
                 span: body.span,
@@ -219,8 +214,11 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
             if context.is_borrow() {
                 if util::is_disaligned(self.tcx, self.body, self.param_env, place) {
                     let source_info = self.source_info;
-                    let lint_root =
+                    let lint_root = self.body.source_scopes[source_info.scope]
-                        self.source_scope_local_data[source_info.scope].lint_root;
+                        .local_data
+                        .as_ref()
+                        .assert_crate_local()
+                        .lint_root;
                     self.register_violations(&[UnsafetyViolation {
                         source_info,
                         description: Symbol::intern("borrow of packed field"),
@@ -346,7 +344,11 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
     fn register_violations(&mut self,
                            violations: &[UnsafetyViolation],
                            unsafe_blocks: &[(hir::HirId, bool)]) {
-        let safety = self.source_scope_local_data[self.source_info.scope].safety;
+        let safety = self.body.source_scopes[self.source_info.scope]
+            .local_data
+            .as_ref()
+            .assert_crate_local()
+            .safety;
         let within_unsafe = match safety {
             // `unsafe` blocks are required in safe code
             Safety::Safe => {
@@ -516,17 +518,6 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: DefId) -> UnsafetyCheckResult
     // `mir_built` force this.
     let body = &tcx.mir_built(def_id).borrow();
 
-    let source_scope_local_data = match body.source_scope_local_data {
-        ClearCrossCrate::Set(ref data) => data,
-        ClearCrossCrate::Clear => {
-            debug!("unsafety_violations: {:?} - remote, skipping", def_id);
-            return UnsafetyCheckResult {
-                violations: Lrc::new([]),
-                unsafe_blocks: Lrc::new([])
-            }
-        }
-    };
-
     let param_env = tcx.param_env(def_id);
 
     let id = tcx.hir().as_local_hir_id(def_id).unwrap();
@@ -536,9 +527,7 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: DefId) -> UnsafetyCheckResult
         hir::BodyOwnerKind::Const |
         hir::BodyOwnerKind::Static(_) => (true, false),
     };
-    let mut checker = UnsafetyChecker::new(
+    let mut checker = UnsafetyChecker::new(const_context, min_const_fn, body, tcx, param_env);
-        const_context, min_const_fn,
-        body, source_scope_local_data, tcx, param_env);
     checker.visit_body(body);
 
     check_unused_unsafe(tcx, def_id, &checker.used_unsafe, &mut checker.inherited_blocks);

src/librustc_mir/transform/const_prop.rs

@@ -9,7 +9,7 @@ use rustc::hir::def_id::DefId;
 use rustc::mir::{
     AggregateKind, Constant, Location, Place, PlaceBase, Body, Operand, Rvalue, Local, UnOp,
     StatementKind, Statement, LocalKind, TerminatorKind, Terminator,  ClearCrossCrate, SourceInfo,
-    BinOp, SourceScope, SourceScopeLocalData, LocalDecl, BasicBlock, RETURN_PLACE,
+    BinOp, SourceScope, SourceScopeData, LocalDecl, BasicBlock, RETURN_PLACE,
 };
 use rustc::mir::visit::{
     Visitor, PlaceContext, MutatingUseContext, MutVisitor, NonMutatingUseContext,
@@ -74,17 +74,10 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
 
         trace!("ConstProp starting for {:?}", source.def_id());
 
-        // Steal some data we need from `body`.
-        let source_scope_local_data = std::mem::replace(
-            &mut body.source_scope_local_data,
-            ClearCrossCrate::Clear
-        );
-
         let dummy_body =
             &Body::new(
                 body.basic_blocks().clone(),
-                Default::default(),
+                body.source_scopes.clone(),
-                ClearCrossCrate::Clear,
                 body.local_decls.clone(),
                 Default::default(),
                 body.arg_count,
@@ -101,19 +94,11 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
         let mut optimization_finder = ConstPropagator::new(
             body,
             dummy_body,
-            source_scope_local_data,
             tcx,
             source
         );
         optimization_finder.visit_body(body);
 
-        // put back the data we stole from `mir`
-        let source_scope_local_data = optimization_finder.release_stolen_data();
-        std::mem::replace(
-            &mut body.source_scope_local_data,
-            source_scope_local_data
-        );
-
         trace!("ConstProp done for {:?}", source.def_id());
     }
 }
@@ -267,7 +252,9 @@ struct ConstPropagator<'mir, 'tcx> {
     source: MirSource<'tcx>,
     can_const_prop: IndexVec<Local, bool>,
     param_env: ParamEnv<'tcx>,
-    source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
+    // FIXME(eddyb) avoid cloning these two fields more than once,
+    // by accessing them through `ecx` instead.
+    source_scopes: IndexVec<SourceScope, SourceScopeData>,
     local_decls: IndexVec<Local, LocalDecl<'tcx>>,
     ret: Option<OpTy<'tcx, ()>>,
 }
@@ -299,7 +286,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     fn new(
         body: &Body<'tcx>,
         dummy_body: &'mir Body<'tcx>,
-        source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
         tcx: TyCtxt<'tcx>,
         source: MirSource<'tcx>,
     ) -> ConstPropagator<'mir, 'tcx> {
@@ -337,17 +323,15 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             source,
             param_env,
             can_const_prop,
-            source_scope_local_data,
+            // FIXME(eddyb) avoid cloning these two fields more than once,
+            // by accessing them through `ecx` instead.
+            source_scopes: body.source_scopes.clone(),
             //FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it
             local_decls: body.local_decls.clone(),
             ret: ret.map(Into::into),
         }
     }
 
-    fn release_stolen_data(self) -> ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>> {
-        self.source_scope_local_data
-    }
-
     fn get_const(&self, local: Local) -> Option<Const<'tcx>> {
         if local == RETURN_PLACE {
             // Try to read the return place as an immediate so that if it is representable as a
@@ -377,14 +361,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         F: FnOnce(&mut Self) -> InterpResult<'tcx, T>,
     {
         self.ecx.tcx.span = source_info.span;
-        let lint_root = match self.source_scope_local_data {
+        // FIXME(eddyb) move this to the `Panic(_)` error case, so that
-            ClearCrossCrate::Set(ref ivs) => {
+        // `f(self)` is always called, and that the only difference when the
-                //FIXME(#51314): remove this check
+        // scope's `local_data` is missing, is that the lint isn't emitted.
-                if source_info.scope.index() >= ivs.len() {
+        let lint_root = match &self.source_scopes[source_info.scope].local_data {
-                    return None;
+            ClearCrossCrate::Set(data) => data.lint_root,
-                }
-                ivs[source_info.scope].lint_root
-            },
             ClearCrossCrate::Clear => return None,
         };
         let r = match f(self) {
@@ -525,8 +506,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
                     let right_size = r.layout.size;
                     let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
                     if r_bits.ok().map_or(false, |b| b >= left_bits as u128) {
-                        let source_scope_local_data = match self.source_scope_local_data {
+                        let lint_root = match &self.source_scopes[source_info.scope].local_data {
-                            ClearCrossCrate::Set(ref data) => data,
+                            ClearCrossCrate::Set(data) => data.lint_root,
                             ClearCrossCrate::Clear => return None,
                         };
                         let dir = if *op == BinOp::Shr {
@@ -534,10 +515,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
                         } else {
                             "left"
                         };
-                        let hir_id = source_scope_local_data[source_info.scope].lint_root;
                         self.tcx.lint_hir(
                             ::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
-                            hir_id,
+                            lint_root,
                             span,
                             &format!("attempt to shift {} with overflow", dir));
                         return None;

src/librustc_mir/transform/inline.rs

@@ -391,9 +391,14 @@ impl Inliner<'tcx> {
                 for mut scope in callee_body.source_scopes.iter().cloned() {
                     if scope.parent_scope.is_none() {
                         scope.parent_scope = Some(callsite.location.scope);
+                        // FIXME(eddyb) is this really needed?
+                        // (also note that it's always overwritten below)
                         scope.span = callee_body.span;
                     }
 
+                    // FIXME(eddyb) this doesn't seem right at all.
+                    // The inlined source scopes should probably be annotated as
+                    // such, but also contain all of the original information.
                     scope.span = callsite.location.span;
 
                     let idx = caller_body.source_scopes.push(scope);

src/librustc_mir/transform/promote_consts.rs

@@ -1081,7 +1081,6 @@ pub fn promote_candidates<'tcx>(
                 // FIXME: maybe try to filter this to avoid blowing up
                 // memory usage?
                 body.source_scopes.clone(),
-                body.source_scope_local_data.clone(),
                 initial_locals,
                 IndexVec::new(),
                 0,