vorner/arc-swap

A way to avoid the need for `AccessConvert`?

steffahn opened this issue · 4 comments

Looks like something like

diff --git a/src/access.rs b/src/access.rs
index be4ddfd..5ffcdaa 100644
--- a/src/access.rs
+++ b/src/access.rs
@@ -112,13 +112,52 @@ pub trait Access<T> {
     fn load(&self) -> Self::Guard;
 }
 
-impl<T, A: Access<T>, P: Deref<Target = A>> Access<T> for P {
+impl<T, A: Access<T> + ?Sized, P: Deref<Target = A>> Access<T> for P {
     type Guard = A::Guard;
     fn load(&self) -> Self::Guard {
         self.deref().load()
     }
 }
 
+impl<T> Access<T> for dyn DynAccess<T> + '_ {
+    type Guard = DynGuard<T>;
+
+    fn load(&self) -> Self::Guard {
+        self.load()
+    }
+}
+
+impl<T> Access<T> for dyn DynAccess<T> + '_ + Send {
+    type Guard = DynGuard<T>;
+
+    fn load(&self) -> Self::Guard {
+        self.load()
+    }
+}
+
+impl<T> Access<T> for dyn DynAccess<T> + '_ + Sync + Send {
+    type Guard = DynGuard<T>;
+
+    fn load(&self) -> Self::Guard {
+        self.load()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    fn _expect_access<T>(_: impl Access<T>) {}
+    fn _test1<T>(x: Box<dyn DynAccess<T> + '_>) {
+        _expect_access(x)
+    }
+    fn _test2<T>(x: Box<dyn DynAccess<T> + '_ + Send>) {
+        _expect_access(x)
+    }
+    fn _test3<T>(x: Box<dyn DynAccess<T> + '_ + Send + Sync>) {
+        _expect_access(x)
+    }
+}
+
 impl<T: RefCnt, S: Strategy<T>> Access<T> for ArcSwapAny<T, S> {
     type Guard = Guard<T, S>;

could avoid the need for the AccessConvert wrapper, right?

Edit: Ah, the actual AccessConvert is somewhat more general, as it allows – say – Box<dyn Foo> for a trait Foo: DynAccess, too. Still, this change could make many use cases of AccessConvert unnecessary, I suppose; and it’s not even a breaking change. (It should e.g. be documented though, if this becomes a PR.)

I think I was trying to come up with something like that back then. I don't know why it failed for me, maybe there were some kind of conflicting implementations. Would you try making it into a PR (maybe without the docs first) to see if it passes all the tests, including on the old but still supported platforms?

@vorner here you go: #78

and it’s not even a breaking change

I mean, one might think it is because the blanket impl gets more general with the added ?Sized; but since rustc doesn’t even appear to like straightforward user-code such as

use arc_swap::access::Access;

trait Foo {}

impl Access<()> for Box<dyn Foo> {
    type Guard = &'static ();
    fn load(&self) -> &'static () {
        &()
    }
}

without complaining about alleged “overlap”, despite the fact that the <Box<dyn Foo> as Deref>::Target is not Sized, makes me think that there might actually be no breakage happening here.

The #78 PR got cherry-picked and merged as part of #85, will be part of the next release.

Thank you