Stop referring to statics' AllocIds directly

Author: oli-obk

src/librustc/ich/impls_ty.rs

@@ -398,8 +398,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::AllocId {
         ty::tls::with_opt(|tcx| {
             trace!("hashing {:?}", *self);
             let tcx = tcx.expect("can't hash AllocIds during hir lowering");
-            if let Some(def_id) = tcx.interpret_interner
-                                     .get_corresponding_static_def_id(*self) {
+            if let Some(def_id) = tcx.interpret_interner.get_static(*self) {
                 AllocDiscriminant::Static.hash_stable(hcx, hasher);
                 trace!("hashing {:?} as static {:?}", *self, def_id);
                 def_id.hash_stable(hcx, hasher);

src/librustc/mir/interpret/mod.rs

@@ -158,7 +158,7 @@ impl ::rustc_serialize::UseSpecializedDecodable for AllocId {}
 enum AllocKind {
     Alloc,
     Fn,
-    ExternStatic,
+    Static,
 }
 
 pub fn specialized_encode_alloc_id<
@@ -173,17 +173,13 @@ pub fn specialized_encode_alloc_id<
         trace!("encoding {:?} with {:#?}", alloc_id, alloc);
         AllocKind::Alloc.encode(encoder)?;
         alloc.encode(encoder)?;
-        // encode whether this allocation is the root allocation of a static
-        tcx.interpret_interner
-            .get_corresponding_static_def_id(alloc_id)
-            .encode(encoder)?;
     } else if let Some(fn_instance) = tcx.interpret_interner.get_fn(alloc_id) {
         trace!("encoding {:?} with {:#?}", alloc_id, fn_instance);
         AllocKind::Fn.encode(encoder)?;
         fn_instance.encode(encoder)?;
-    } else if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) {
-        // extern "C" statics don't have allocations, just encode its def_id
-        AllocKind::ExternStatic.encode(encoder)?;
+    } else if let Some(did) = tcx.interpret_interner.get_static(alloc_id) {
+        // referring to statics doesn't need to know about their allocations, just hash the DefId
+        AllocKind::Static.encode(encoder)?;
         did.encode(encoder)?;
     } else {
         bug!("alloc id without corresponding allocation: {}", alloc_id);
@@ -212,10 +208,6 @@ pub fn specialized_decode_alloc_id<
             let allocation = tcx.intern_const_alloc(allocation);
             tcx.interpret_interner.intern_at_reserved(alloc_id, allocation);
 
-            if let Some(glob) = Option::<DefId>::decode(decoder)? {
-                tcx.interpret_interner.cache(glob, alloc_id);
-            }
-
             Ok(alloc_id)
         },
         AllocKind::Fn => {
@@ -227,12 +219,11 @@ pub fn specialized_decode_alloc_id<
             cache(decoder, id);
             Ok(id)
         },
-        AllocKind::ExternStatic => {
+        AllocKind::Static => {
             trace!("creating extern static alloc id at");
             let did = DefId::decode(decoder)?;
-            let alloc_id = tcx.interpret_interner.reserve();
+            let alloc_id = tcx.interpret_interner.cache_static(did);
             cache(decoder, alloc_id);
-            tcx.interpret_interner.cache(did, alloc_id);
             Ok(alloc_id)
         },
     }

src/librustc/ty/context.rs

@@ -935,18 +935,16 @@ struct InterpretInternerInner<'tcx> {
     /// Allows obtaining const allocs via a unique identifier
     alloc_by_id: FxHashMap<interpret::AllocId, &'tcx interpret::Allocation>,
 
-    /// Reverse map of `alloc_cache`
-    global_cache: FxHashMap<interpret::AllocId, DefId>,
+    /// Allows obtaining static def ids via a unique id
+    statics: FxHashMap<interpret::AllocId, DefId>,
 
     /// The AllocId to assign to the next new regular allocation.
     /// Always incremented, never gets smaller.
     next_id: interpret::AllocId,
 
-    /// Allows checking whether a static already has an allocation
-    ///
-    /// This is only important for detecting statics referring to themselves
-    // FIXME(oli-obk) move it to the EvalContext?
-    alloc_cache: FxHashMap<DefId, interpret::AllocId>,
+    /// Inverse map of `statics`
+    /// Used so we don't allocate a new pointer every time we need one
+    static_cache: FxHashMap<DefId, interpret::AllocId>,
 
     /// A cache for basic byte allocations keyed by their contents. This is used to deduplicate
     /// allocations for string and bytestring literals.
@@ -980,30 +978,25 @@ impl<'tcx> InterpretInterner<'tcx> {
         self.inner.borrow().alloc_by_id.get(&id).cloned()
     }
 
-    pub fn get_cached(
-        &self,
-        static_id: DefId,
-    ) -> Option<interpret::AllocId> {
-        self.inner.borrow().alloc_cache.get(&static_id).cloned()
-    }
-
-    pub fn cache(
+    pub fn cache_static(
         &self,
         static_id: DefId,
-        alloc_id: interpret::AllocId,
-    ) {
-        let mut inner = self.inner.borrow_mut();
-        inner.global_cache.insert(alloc_id, static_id);
-        if let Some(old) = inner.alloc_cache.insert(static_id, alloc_id) {
-            bug!("tried to cache {:?}, but was already existing as {:#?}", static_id, old);
+    ) -> interpret::AllocId {
+        if let Some(alloc_id) = self.inner.borrow().static_cache.get(&static_id).cloned() {
+            return alloc_id;
         }
+        let alloc_id = self.reserve();
+        let mut inner = self.inner.borrow_mut();
+        inner.static_cache.insert(static_id, alloc_id);
+        inner.statics.insert(alloc_id, static_id);
+        alloc_id
     }
 
-    pub fn get_corresponding_static_def_id(
+    pub fn get_static(
         &self,
         ptr: interpret::AllocId,
     ) -> Option<DefId> {
-        self.inner.borrow().global_cache.get(&ptr).cloned()
+        self.inner.borrow().statics.get(&ptr).cloned()
     }
 
     pub fn intern_at_reserved(

src/librustc_mir/interpret/const_eval.rs

@@ -5,7 +5,6 @@ use rustc::mir;
 use rustc::ty::{self, TyCtxt, Ty, Instance};
 use rustc::ty::layout::{self, LayoutOf};
 use rustc::ty::subst::Subst;
-use rustc::util::nodemap::FxHashSet;
 
 use syntax::ast::Mutability;
 use syntax::codemap::Span;
@@ -110,53 +109,38 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>(
         }
         span = mir.span;
         let layout = ecx.layout_of(mir.return_ty().subst(tcx, cid.instance.substs))?;
-        let alloc = tcx.interpret_interner.get_cached(cid.instance.def_id());
-        let is_static = tcx.is_static(cid.instance.def_id()).is_some();
-        let alloc = match alloc {
-            Some(alloc) => {
-                assert!(cid.promoted.is_none());
-                assert!(param_env.caller_bounds.is_empty());
-                alloc
-            },
-            None => {
-                assert!(!layout.is_unsized());
-                let ptr = ecx.memory.allocate(
-                    layout.size.bytes(),
-                    layout.align,
-                    None,
-                )?;
-                if is_static {
-                    tcx.interpret_interner.cache(cid.instance.def_id(), ptr.alloc_id);
-                }
-                let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span);
-                let mutability = tcx.is_static(cid.instance.def_id());
-                let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable {
-                    Mutability::Mutable
-                } else {
-                    Mutability::Immutable
-                };
-                let cleanup = StackPopCleanup::MarkStatic(mutability);
-                let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id()));
-                let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p));
-                trace!("const_eval: pushing stack frame for global: {}{}", name, prom);
-                assert!(mir.arg_count == 0);
-                ecx.push_stack_frame(
-                    cid.instance,
-                    mir.span,
-                    mir,
-                    Place::from_ptr(ptr, layout.align),
-                    cleanup,
-                )?;
-
-                while ecx.step()? {}
-                ptr.alloc_id
-            }
+        assert!(!layout.is_unsized());
+        let ptr = ecx.memory.allocate(
+            layout.size.bytes(),
+            layout.align,
+            None,
+        )?;
+        let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span);
+        let mutability = tcx.is_static(cid.instance.def_id());
+        let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable {
+            Mutability::Mutable
+        } else {
+            Mutability::Immutable
         };
-        let ptr = MemoryPointer::new(alloc, 0).into();
+        let cleanup = StackPopCleanup::MarkStatic(mutability);
+        let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id()));
+        let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p));
+        trace!("const_eval: pushing stack frame for global: {}{}", name, prom);
+        assert!(mir.arg_count == 0);
+        ecx.push_stack_frame(
+            cid.instance,
+            mir.span,
+            mir,
+            Place::from_ptr(ptr, layout.align),
+            cleanup,
+        )?;
+
+        while ecx.step()? {}
+        let ptr = ptr.into();
         // always try to read the value and report errors
         let value = match ecx.try_read_value(ptr, layout.align, layout.ty)? {
             // if it's a constant (so it needs no address, directly compute its value)
-            Some(val) if !is_static => val,
+            Some(val) if tcx.is_static(cid.instance.def_id()).is_none() => val,
             // point at the allocation
             _ => Value::ByRef(ptr, layout.align),
         };
@@ -340,21 +324,10 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator {
         ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
         cid: GlobalId<'tcx>,
     ) -> EvalResult<'tcx, AllocId> {
-        let alloc = ecx
-                    .tcx
-                    .interpret_interner
-                    .get_cached(cid.instance.def_id());
-        // Don't evaluate when already cached to prevent cycles
-        if let Some(alloc) = alloc {
-            return Ok(alloc)
-        }
-        // ensure the static is computed
-        ecx.const_eval(cid)?;
         Ok(ecx
             .tcx
             .interpret_interner
-            .get_cached(cid.instance.def_id())
-            .expect("uncached static"))
+            .cache_static(cid.instance.def_id()))
     }
 
     fn box_alloc<'a>(
@@ -460,16 +433,7 @@ pub fn const_eval_provider<'a, 'tcx>(
     let def_id = cid.instance.def.def_id();
 
     if tcx.is_foreign_item(def_id) {
-        let id = tcx.interpret_interner.get_cached(def_id);
-        let id = match id {
-            // FIXME: due to caches this shouldn't happen, add some assertions
-            Some(id) => id,
-            None => {
-                let id = tcx.interpret_interner.reserve();
-                tcx.interpret_interner.cache(def_id, id);
-                id
-            },
-        };
+        let id = tcx.interpret_interner.cache_static(def_id);
         let ty = tcx.type_of(def_id);
         let layout = tcx.layout_of(key.param_env.and(ty)).unwrap();
         let ptr = MemoryPointer::new(id, 0);
@@ -505,13 +469,7 @@ pub fn const_eval_provider<'a, 'tcx>(
     };
 
     let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env);
-    res.map(|(miri_value, ptr, miri_ty)| {
-        if tcx.is_static(def_id).is_some() {
-            if let Ok(ptr) = ptr.primval.to_ptr() {
-                let mut seen = FxHashSet::default();
-                create_depgraph_edges(tcx, ptr.alloc_id, &mut seen);
-            }
-        }
+    res.map(|(miri_value, _, miri_ty)| {
         tcx.mk_const(ty::Const {
             val: ConstVal::Value(miri_value),
             ty: miri_ty,
@@ -528,35 +486,3 @@ pub fn const_eval_provider<'a, 'tcx>(
         }
     })
 }
-
-// This function creates dep graph edges from statics to all referred to statics.
-// This is necessary, because the `const_eval` query cannot directly call itself
-// for other statics, because we cannot prevent recursion in queries.
-//
-// see test/incremental/static_refering_to_other_static2/issue.rs for an example
-// where not creating those edges would cause static A, which refers to static B
-// to point to the old allocation of static B, even though B has changed.
-//
-// In the future we will want to remove this funcion in favour of a system that
-// makes sure that statics don't need to have edges to other statics as long as
-// they are only referring by reference and not inspecting the other static's body.
-fn create_depgraph_edges<'a, 'tcx>(
-    tcx: TyCtxt<'a, 'tcx, 'tcx>,
-    alloc_id: AllocId,
-    seen: &mut FxHashSet<AllocId>,
-) {
-    trace!("create_depgraph_edges: {:?}, {:?}", alloc_id, seen);
-    if seen.insert(alloc_id) {
-        trace!("seen: {:?}, {:?}", alloc_id, seen);
-        if let Some(alloc) = tcx.interpret_interner.get_alloc(alloc_id) {
-            trace!("get_alloc: {:?}, {:?}, {:?}", alloc_id, seen, alloc);
-            for (_, &reloc) in &alloc.relocations {
-                if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(reloc) {
-                    trace!("get_corresponding: {:?}, {:?}, {:?}, {:?}, {:?}", alloc_id, seen, alloc, did, reloc);
-                    let _ = tcx.maybe_optimized_mir(did);
-                }
-                create_depgraph_edges(tcx, reloc, seen);
-            }
-        }
-    }
-}

src/librustc_mir/interpret/eval_context.rs

@@ -929,16 +929,14 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
     }
 
     pub fn read_global_as_value(&self, gid: GlobalId<'tcx>, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
-        if gid.promoted.is_none() {
-            let cached = self
+        if self.tcx.is_static(gid.instance.def_id()).is_some() {
+            let alloc_id = self
                 .tcx
                 .interpret_interner
-                .get_cached(gid.instance.def_id());
-            if let Some(alloc_id) = cached {
-                let layout = self.layout_of(ty)?;
-                let ptr = MemoryPointer::new(alloc_id, 0);
-                return Ok(Value::ByRef(ptr.into(), layout.align))
-            }
+                .cache_static(gid.instance.def_id());
+            let layout = self.layout_of(ty)?;
+            let ptr = MemoryPointer::new(alloc_id, 0);
+            return Ok(Value::ByRef(ptr.into(), layout.align))
         }
         let cv = self.const_eval(gid)?;
         self.const_to_value(&cv.val, ty)

src/librustc_mir/monomorphize/collector.rs

@@ -1111,7 +1111,7 @@ fn collect_miri<'a, 'tcx>(
     alloc_id: AllocId,
     output: &mut Vec<MonoItem<'tcx>>,
 ) {
-    if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) {
+    if let Some(did) = tcx.interpret_interner.get_static(alloc_id) {
         let instance = Instance::mono(tcx, did);
         if should_monomorphize_locally(tcx, &instance) {
             trace!("collecting static {:?}", did);

src/librustc_trans/mir/constant.rs

@@ -50,7 +50,7 @@ pub fn primval_to_llvm(cx: &CodegenCx,
                 let static_ = cx
                     .tcx
                     .interpret_interner
-                    .get_corresponding_static_def_id(ptr.alloc_id);
+                    .get_static(ptr.alloc_id);
                 let base_addr = if let Some(def_id) = static_ {
                     assert!(cx.tcx.is_static(def_id).is_some());
                     consts::get_static(cx, def_id)
@@ -126,18 +126,17 @@ pub fn trans_static_initializer<'a, 'tcx>(
         promoted: None
     };
     let param_env = ty::ParamEnv::reveal_all();
-    cx.tcx.const_eval(param_env.and(cid))?;
+    let static_ = cx.tcx.const_eval(param_env.and(cid))?;
 
-    let alloc_id = cx
-        .tcx
-        .interpret_interner
-        .get_cached(def_id)
-        .expect("global not cached");
+    let ptr = match static_.val {
+        ConstVal::Value(MiriValue::ByRef(ptr, _)) => ptr,
+        _ => bug!("static const eval returned {:#?}", static_),
+    };
 
     let alloc = cx
         .tcx
         .interpret_interner
-        .get_alloc(alloc_id)
+        .get_alloc(ptr.primval.to_ptr().expect("static has integer pointer").alloc_id)
         .expect("miri allocation never successfully created");
     Ok(global_initializer(cx, alloc))
 }