Address reviewers' comments: replace resolve_thread_local_allocation_id with resolve_maybe_global_alloc, clarify comments.

2 files changed, 28 insertions, 22 deletions

diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
index 14acaf82607..e68d92555db 100644
--- a/src/librustc_mir/interpret/machine.rs
+++ b/src/librustc_mir/interpret/machine.rs
@@ -6,7 +6,7 @@ use std::borrow::{Borrow, Cow};
 use std::hash::Hash;
 
 use rustc_middle::mir;
-use rustc_middle::ty::{self, Ty};
+use rustc_middle::ty::{self, query::TyCtxtAt, Ty};
 use rustc_span::def_id::DefId;
 
 use super::{
@@ -229,10 +229,22 @@ pub trait Machine<'mir, 'tcx>: Sized {
         Ok(())
     }
 
-    /// Called for *every* memory access to determine the real ID of the given allocation.
-    /// This provides a way for the machine to "redirect" certain allocations as it sees fit.
+    /// Called for *every* memory access to determine the real ID of the given
+    /// allocation. This provides a way for the machine to "redirect" certain
+    /// allocations as it sees fit.
     ///
-    /// This is used by Miri to redirect extern statics to real allocations.
+    /// This is used by Miri for two purposes:
+    /// 1.  Redirecting extern statics to real allocations.
+    /// 2.  Creating unique allocation ids for thread locals.
+    ///
+    ///     In Rust, one way for creating a thread local is by marking a static
+    ///     with `#[thread_local]`. On supported platforms this gets translated
+    ///     to a LLVM thread local. The problem with supporting these thread
+    ///     locals in Miri is that in the internals of the compiler they look as
+    ///     normal statics, except that they have the `thread_local` attribute.
+    ///     However, in Miri we want to have a property that each allocation has
+    ///     a unique id and, therefore, for these thread locals we generate a
+    ///     fresh allocation id for each thread.
     ///
     /// This function must be idempotent.
     #[inline]
@@ -240,18 +252,18 @@ pub trait Machine<'mir, 'tcx>: Sized {
         id
     }
 
-    /// In Rust, thread locals are just special statics. Therefore, the compiler
-    /// uses the same code for allocating both. However, in Miri we want to have
-    /// a property that each allocation has a unique id and, therefore, we
-    /// generate a fresh allocation id for each thread. This function takes a
-    /// potentially thread local allocation id and resolves the original static
-    /// allocation id that can be used to compute the value of the static.
-    #[inline]
-    fn resolve_thread_local_allocation_id(
+    /// Called to obtain the `GlobalAlloc` associated with the allocation id.
+    ///
+    /// Miri uses this callback to resolve the information about the original
+    /// thread local static for which `canonical_alloc_id` generated a fresh
+    /// allocation id.
+    #[inline(always)]
+    fn resolve_maybe_global_alloc(
+        tcx: TyCtxtAt<'tcx>,
         _memory_extra: &Self::MemoryExtra,
         id: AllocId,
-    ) -> AllocId {
-        id
+    ) -> Option<mir::interpret::GlobalAlloc<'tcx>> {
+        tcx.alloc_map.lock().get(id)
     }
 
     /// Called to initialize the "extra" state of an allocation and make the pointers
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 20158b9c1b6..817d69d2878 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -429,9 +429,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         id: AllocId,
         is_write: bool,
     ) -> InterpResult<'tcx, Cow<'tcx, Allocation<M::PointerTag, M::AllocExtra>>> {
-        let alloc =
-            tcx.alloc_map.lock().get(M::resolve_thread_local_allocation_id(memory_extra, id));
-        let (alloc, def_id) = match alloc {
+        let (alloc, def_id) = match M::resolve_maybe_global_alloc(tcx, memory_extra, id) {
             Some(GlobalAlloc::Memory(mem)) => {
                 // Memory of a constant or promoted or anonymous memory referenced by a static.
                 (mem, None)
@@ -591,11 +589,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         }
 
         // # Statics
-        // Can't do this in the match argument, we may get cycle errors since the lock would
-        // be held throughout the match.
-        let alloc =
-            self.tcx.alloc_map.lock().get(M::resolve_thread_local_allocation_id(&self.extra, id));
-        match alloc {
+        match M::resolve_maybe_global_alloc(self.tcx, &self.extra, id) {
             Some(GlobalAlloc::Static(did)) => {
                 // Use size and align of the type.
                 let ty = self.tcx.type_of(did);