Fix soundness hole around access::Map

The assumption that the address of access's guarded reference stays the
same is not true.

Costs:
* The Map is now slower and adds an allocation.
* It no longer can be Clone (but non-trivial guards weren't anyway) and
  it can stop being Sync/Send if the closure is not.

Fixes #45.

Technically, it is a breaking change, but the plan is not to raise major
version, because:

* Even rust std gives exception to break compatibility for soundness
  hole fixes.
* It is not that likely people's code would break.
* Even if it breaks, they are much more likely to go to the fixed
  version then if the version got bumped and that's what they should be
  doing ASAP due to the potential UB.

src/access.rs

@@ -1,3 +1,5 @@
+#![deny(unsafe_code)]
+
 //! Abstracting over accessing parts of stored value.
 //!
 //! Sometimes, there's a big globalish data structure (like a configuration for the whole program).
@@ -204,51 +206,32 @@ where
 }
 
 #[doc(hidden)]
-#[derive(Copy, Clone, Debug)]
-pub struct MapGuard<G, T> {
-    _guard: G,
-    value: *const T,
-}
-
-// Why these are safe:
-// * The *const T is actually used just as a &const T with 'self lifetime (which can't be done in
-//   Rust). So if the reference is Send/Sync, so is the raw pointer.
-unsafe impl<G, T> Send for MapGuard<G, T>
-where
-    G: Send,
-    for<'a> &'a T: Send,
-{
+#[derive(Clone, Debug)]
+pub struct MapGuard<G, F, T, R> {
+    guard: G,
+    projection: Arc<F>,
+    _t: PhantomData<fn(&T) -> &R>,
 }
 
-unsafe impl<G, T> Sync for MapGuard<G, T>
+impl<G, F, T, R> Deref for MapGuard<G, F, T, R>
 where
-    G: Sync,
-    for<'a> &'a T: Sync,
+    G: Deref<Target = T>,
+    F: Fn(&T) -> &R,
 {
-}
-
-impl<G, T> Deref for MapGuard<G, T> {
-    type Target = T;
-    fn deref(&self) -> &T {
-        // Why this is safe:
-        // * The pointer is originally converted from a reference. It's not null, it's aligned,
-        //   it's the right type, etc.
-        // * The pointee couldn't have gone away ‒ the guard keeps the original reference alive, so
-        //   must the new one still be alive too. Moving the guard is fine, we assume the RefCnt is
-        //   Pin (because it's Arc or Rc or something like that ‒ when that one moves, the data it
-        //   points to stay at the same place).
-        unsafe { &*self.value }
+    type Target = R;
+    fn deref(&self) -> &R {
+        (self.projection)(&self.guard)
     }
 }
 
 /// An adaptor to provide access to a part of larger structure.
 ///
 /// This is the *active* part of this module. Use the [module documentation](index.html) for the
 /// details.
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct Map<A, T, F> {
     access: A,
-    projection: F,
+    projection: Arc<F>,
     _t: PhantomData<fn() -> T>,
 }
 
@@ -270,24 +253,24 @@ impl<A, T, F> Map<A, T, F> {
     {
         Map {
             access,
-            projection,
+            projection: Arc::new(projection),
             _t: PhantomData,
         }
     }
 }
 
-impl<A, T, F, R> Access<R> for Map<A, T, F>
+impl<A, F, T, R> Access<R> for Map<A, T, F>
 where
     A: Access<T>,
     F: Fn(&T) -> &R,
 {
-    type Guard = MapGuard<A::Guard, R>;
+    type Guard = MapGuard<A::Guard, F, T, R>;
     fn load(&self) -> Self::Guard {
         let guard = self.access.load();
-        let value: *const _ = (self.projection)(&guard);
         MapGuard {
-            _guard: guard,
-            value,
+            guard,
+            projection: Arc::clone(&self.projection),
+            _t: PhantomData,
         }
     }
 }