Eq instance is shady #143

Activity

1. treeowl commented on Mar 3, 2023

   

   treeowl

   on Mar 3, 2023

   ContributorAuthor

   More actions

   We could have a SomeAsync with legitimate instances.

   The simple way:

   data SomeAsync = forall a. SomeAsync (Async a)
   instance Ord SomeAsync where
     compare (SomeAsync (Async t1 _)) (SomeAsync (Async t2 _)) = compare t1 t2

   Alternatively, we could do this:

   -- Any instead of a proper existential so this can unpack
   data Async a = Async
     { asyncThreadId :: !ThreadId
     , _resVar :: !(TMVar (Either SomeException Any))
     , _resMod :: (Any -> a) }
   
   instance Functor Async where
     fmap f (Async tid rv g) = Async tid rv (f . g)
     -- Note that <$ replaces the function instead of adding onto it
     x <$ Async tid rv _ = Async tid rv (const x)
   
   data SomeAsync = SomeAsync_ !ThreadId !(TMVar (Either SomeException Any))
   instance Ord SomeAsync where
     compare (SomeAsync tid1 _) (SomeAsync tid2 _) = compare tid1 tid2
   
   pattern SomeAsync :: MyAsync a -> SomeAsync
   pattern SomeAsync x <- ((\(SomeAsync_ tid mv) -> MyAsync tid mv (const ())) -> x)
     where
       SomeAsync (MyAsync tid tmv _) = SomeAsync_ tid tmv

   Another variation on that last is to make SomeAsync a newtype around Async, using the tricks in the some package. That could be good for things like cancelMany.
2. simonmar commented on Mar 3, 2023

   

   simonmar

   on Mar 3, 2023

   Owner

   More actions

   Not a big fan of SomeAsync, why not just have sameAsync :: Async a -> Async b -> Bool ?
3. treeowl commented on Mar 3, 2023

   

   treeowl

   on Mar 3, 2023

   ContributorAuthor

   More actions

   We should have that, yes, but I think sometimes people want to be able to do things like use Asyncs for Set or Map keys for whatever reason. SomeAsync, by any implementation, allows that.
4. simonmar commented on Mar 3, 2023

   

   simonmar

   on Mar 3, 2023

   Owner

   More actions

   Ok, fair point.
5. treeowl commented on Mar 3, 2023

   

   treeowl

   on Mar 3, 2023

   ContributorAuthor

   More actions

   cancelMany would also be more useful if it took [SomeAsync] instead of [Async a]. Do you have an opinion about which SomeAsync representation to use? I'm partial to the approach of separating the TMVar from the mapping function myself—while that makes the Async heap object one word larger, it reduces the size of the function closure and allows for a more efficient <$>.
6. simonmar commented on Sep 3, 2023

   

   simonmar

   on Sep 3, 2023

   Owner

   More actions

   Not to leave this hanging indefinitely - basically I consider this "bug" to be something of a technicality. If we fix it, the cure better not be worse than the disease, and the disease is pretty mild IMO.