data_structures: Add a reference wrapper for pointer-indexed maps/sets

Use `ptr::eq` for comparing pointers
rust-lang · bors · Jul 21, 2018 · Jul 16, 2018 · Jul 16, 2018 · Jul 17, 2018
commit 2eb83ee527fdb4bcd740ea41c43e9d2d52f99c1c
diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs
@@ -103,7 +103,7 @@ impl<K, V> LeafNode<K, V> {
     }
 
     fn is_shared_root(&self) -> bool {
-        self as *const _ == &EMPTY_ROOT_NODE as *const _ as *const LeafNode<K, V>
+        ptr::eq(self, &EMPTY_ROOT_NODE as *const _ as *const _)
     }
 }
 

diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs
@@ -39,7 +39,7 @@ use hir
8000
::intravisit;
 use hir;
 use lint::builtin::BuiltinLintDiagnostics;
 use session::{Session, DiagnosticMessageId};
-use std::hash;
+use std::{hash, ptr};
 use syntax::ast;
 use syntax::codemap::MultiSpan;
 use syntax::edition::Edition;
@@ -354,7 +354,7 @@ pub struct LintId {
 
 impl PartialEq for LintId {
     fn eq(&self, other: &LintId) -> bool {
-        (self.lint as *const Lint) == (other.lint as *const Lint)
+        ptr::eq(self.lint, other.lint)
     }
 }
 

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
@@ -47,7 +47,7 @@ use std::ops::Deref;
 use rustc_data_structures::sync::{self, Lrc, ParallelIterator, par_iter};
 use std::slice;
 use std::vec::IntoIter;
-use std::mem;
+use std::{mem, ptr};
 use syntax::ast::{self, DUMMY_NODE_ID, Name, Ident, NodeId};
 use syntax::attr;
 use syntax::ext::hygiene::Mark;
@@ -527,8 +527,7 @@ impl<'tcx> PartialOrd for TyS<'tcx> {
 impl<'tcx> PartialEq for TyS<'tcx> {
     #[inline]
     fn eq(&self, other: &TyS<'tcx>) -> bool {
-        // (self as *const _) == (other as *const _)
-        (self as *const TyS<'tcx>) == (other as *const TyS<'tcx>)
+        ptr::eq(self, other)
     }
 }
 impl<'tcx> Eq for TyS<'tcx> {}
@@ -678,7 +677,7 @@ impl<T> PartialOrd for Slice<T> where T: PartialOrd {
 impl<T: PartialEq> PartialEq for Slice<T> {
     #[inline]
     fn eq(&self, other: &Slice<T>) -> bool {
-        (self as *const _) == (other as *const _)
+        ptr::eq(self, other)
     }
 }
 impl<T: Eq> Eq for Slice<T> {}
@@ -1730,7 +1729,7 @@ impl Ord for AdtDef {
 impl PartialEq for AdtDef {
     // AdtDef are always interned and this is part of TyS equality
     #[inline]
-    fn eq(&self, other: &Self) -> bool { self as *const _ == other as *const _ }
+    fn eq(&self, other: &Self) -> bool { ptr::eq(self, other) }
 }
 
 impl Eq for AdtDef {}

diff --git a/src/librustc/ty/query/job.rs b/src/librustc/ty/query/job.rs
@@ -11,6 +11,7 @@
 #![allow(warnings)]
 
 use std::mem;
+use rustc_data_structures::ptr_key::PtrKey;
 use rustc_data_structures::sync::{Lock, LockGuard, Lrc, Weak};
 use rustc_data_structures::OnDrop;
 use syntax_pos::Span;
@@ -20,7 +21,7 @@ use ty::query::plumbing::CycleError;
 use ty::context::TyCtxt;
 use errors::Diagnostic;
 use std::process;
-use std::fmt;
+use std::{fmt, ptr};
 use std::collections::HashSet;
 #[cfg(parallel_queries)]
 use {
@@ -124,7 +125,7 @@ impl<'tcx> QueryJob<'tcx> {
         while let Some(job) = current_job {
             cycle.insert(0, job.info.clone());
 
-            if &*job as *const _ == self as *const _ {
+            if ptr::eq(&*job, self) {
                 // This is the end of the cycle
                 // The span entry we included was for the usage
                 // of the cycle itself, and not part of the cycle
@@ -282,7 +283,7 @@ where
 fn cycle_check<'tcx>(query: Lrc<QueryJob<'tcx>>,
                      span: Span,
                      stack: &mut Vec<(Span, Lrc<QueryJob<'tcx>>)>,
-                     visited: &mut HashSet<*const QueryJob<'tcx>>
+                     visited: &mut HashSet<PtrKey<'tcx, QueryJob<'tcx>>>
 ) -> Option<Option<Waiter<'tcx>>> {
     if visited.contains(&query.as_ptr()) {
         return if let Some(p) = stack.iter().position(|q| q.1.as_ptr() == query.as_ptr()) {
@@ -321,7 +322,7 @@ fn cycle_check<'tcx>(query: Lrc<QueryJob<'tcx>>,
 #[cfg(parallel_queries)]
 fn connected_to_root<'tcx>(
     query: Lrc<QueryJob<'tcx>>,
-    visited: &mut HashSet<*const QueryJob<'tcx>>
+    visited: &mut HashSet<PtrKey<'tcx, QueryJob<'tcx>>>
 ) -> bool {
     // We already visited this or we're deliberately ignoring it
     if visited.contains(&query.as_ptr()) {

diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs
@@ -56,29 +56,30 @@ extern crate rustc_cratesio_shim;
 
 pub use rustc_serialize::hex::ToHex;
 
-pub mod array_vec;
 pub mod accumulate_vec;
-pub mod small_vec;
+pub mod array_vec;
 pub mod base_n;
 pub mod bitslice;
 pub mod bitvec;
+pub mod flock;
+pub mod fx;
+pub mod graph;
 pub mod indexed_set;
 pub mod indexed_vec;
 pub mod obligation_forest;
+pub mod owning_ref;
+pub mod ptr_key;
 pub mod sip128;
+pub mod small_vec;
 pub mod snapshot_map;
 pub use ena::snapshot_vec;
+pub mod sorted_map;
 pub mod stable_hasher;
-pub mod transitive_relation;
-pub use ena::unify;
-pub mod fx;
-pub mod tuple_slice;
-pub mod graph;
-pub mod flock;
 pub mod sync;
-pub mod owning_ref;
 pub mod tiny_list;
-pub mod sorted_map;
+pub mod transitive_relation;
+pub mod tuple_slice;
+pub use ena::unify;
 pub mod work_queue;
 
 pub struct OnDrop<F: Fn()>(pub F);

diff --git a/src/librustc_data_structures/ptr_key.rs b/src/librustc_data_structures/ptr_key.rs
@@ -0,0 +1,45 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::{hash, ptr};
+use std::ops::Deref;
+
+/// A wrapper around reference that compares and hashes like a pointer.
+/// Can be used as a key in sets/maps indexed by pointers to avoid `unsafe`.
+#[derive(Debug)]
+pub struct PtrKey<'a, T: 'a>(pub &'a T);
+
+impl<'a, T> Clone for PtrKey<'a, T> {
+    fn clone(&self) -> Self { *self }
+}
+
+impl<'a, T> Copy for PtrKey<'a, T> {}
+
+impl<'a, T> PartialEq for PtrKey<'a, T> {
+    fn eq(&self, rhs: &Self) -> bool {
+        ptr::eq(self.0, rhs.0)
+    }
+}
+
+impl<'a, T> Eq for PtrKey<'a, T> {}
+
+impl<'a, T> hash::Hash for PtrKey<'a, T> {
+    fn hash<H: hash::Hasher>(&self, hasher: &mut H) {
+        (self.0 as *const T).hash(hasher)
+    }
+}
+
+impl<'a, T> Deref for PtrKey<'a, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -17,6 +17,7 @@ use Resolver;
 use {names_to_string, module_to_string};
 use {resolve_error, ResolutionError};
 
+use rustc_data_structures::ptr_key::PtrKey;
 use rustc::ty;
 use rustc::lint::builtin::BuiltinLintDiagnostics;
 use rustc::lint::builtin::{DUPLICATE_MACRO_EXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE};
@@ -33,7 +34,7 @@ use syntax::util::lev_distance::find_best_match_for_name;
 use syntax_pos::Span;
 
 use std::cell::{Cell, RefCell};
-use std::mem;
+use std::{mem, ptr};
 
 /// Contains data for specific types of import directives.
 #[derive(Clone, Debug)]
@@ -105,8 +106,8 @@ impl<'a> ImportDirective<'a> {
 /// Records information about the resolution of a name in a namespace of a module.
 pub struct NameResolution<'a> {
     /// Single imports that may define the name in the namespace.
-    /// Import directives are arena-allocated, so it's ok to use pointers as keys, they are stable.
-    single_imports: FxHashSet<*const ImportDirective<'a>>,
+    /// Import directives are arena-allocated, so it's ok to use pointers as keys.
+    single_imports: FxHashSet<PtrKey<'a, ImportDirective<'a>>>,
     /// The least shadowable known binding for this name, or None if there are no known bindings.
     pub binding: Option<&'a NameBinding<'a>>,
     shadowed_glob: Option<&'a NameBinding<'a>>,
@@ -192,7 +193,6 @@ impl<'a> Resolver<'a> {
         // Check if one of single imports can still define the name,
         // if it can then our result is not determined and can be invalidated.
         for single_import in &resolution.single_imports {
-            let single_import = unsafe { &**single_import };
             if !self.is_accessible(single_import.vis.get()) {
                 continue;
             }
@@ -291,7 +291,7 @@ impl<'a> Resolver<'a> {
             SingleImport { target, type_ns_only, .. } => {
                 self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
                     let mut resolution = this.resolution(current_module, target, ns).borrow_mut();
-                    resolution.single_imports.insert(directive);
+                    resolution.single_imports.insert(PtrKey(directive));
                 });
             }
             // We don't add prelude imports to the globs since they only affect lexical scopes,
@@ -398,7 +398,7 @@ impl<'a> Resolver<'a> {
                 _ if old_binding.is_some() => return t,
                 None => return t,
                 Some(binding) => match old_binding {
-                    Some(old_binding) if old_binding as *const _ == binding as *const _ => return t,
+                    Some(old_binding) if ptr::eq(old_binding, binding) => return t,
                     _ => (binding, t),
                 }
             }
@@ -583,7 +583,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                 Err(Undetermined) => indeterminate = true,
                 Err(Determined) => {
                     this.update_resolution(parent, target, ns, |_, resolution| {
-                        resolution.single_imports.remove(&(directive as *const _));
+                        resolution.single_imports.remove(&PtrKey(directive));
                     });
                 }
                 Ok(binding) if !binding.is_importable() => {
@@ -916,7 +916,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
 
         let mut reexports = Vec::new();
         let mut exported_macro_names = FxHashMap();
-        if module as *const _ == self.graph_root as *const _ {
+        if ptr::eq(module, self.graph_root) {
             let macro_exports = mem::replace(&mut self.macro_exports, Vec::new());
             for export in macro_exports.into_iter().rev() {
                 if let Some(later_span) = exported_macro_names.insert(export.ident.modern(),
-Original file line number
+Diff line change
@@ Expand Up / @@ -103,7 +103,7 @@ impl<K, V> LeafNode<K, V> { @@
         }
         fn is_shared_root(&self) -> bool {
-            self as *const _ == &EMPTY_ROOT_NODE as *const _ as *const LeafNode<K, V>
+            ptr::eq(self, &EMPTY_ROOT_NODE as *const _ as *const _)
         }
     }
@@ Expand Down @@