Skip to content

Draft: Give a better implementation for (*>)#546

Open
L0neGamer wants to merge 2 commits intohedgehogqa:masterfrom
L0neGamer:better-then
Open

Draft: Give a better implementation for (*>)#546
L0neGamer wants to merge 2 commits intohedgehogqa:masterfrom
L0neGamer:better-then

Conversation

@L0neGamer
Copy link
Copy Markdown

@L0neGamer L0neGamer commented May 19, 2025

~~The default implementation of (*>) uses (<*>) which tries to perform each branch in parallel. However, since we only get the value from the right argument, we should just return that argument.

I don't implement (>>) similarly due to my attempts with haskell/core-libraries-committee#333, but if that proposal falls through I'll be back.~~

Simply cleanup some instance definitions and add a comment.

@L0neGamer L0neGamer changed the title encode a better *> operator Give a better implementation for (*>) May 19, 2025
@ocharles
Copy link
Copy Markdown
Contributor

This doesn't look sound - what if I'm using GenT State? You're not going to run the left potentially-stateful computation at all!

@L0neGamer
Copy link
Copy Markdown
Author

Damn, you've a point... I'll have another go

@L0neGamer L0neGamer changed the title Give a better implementation for (*>) Draft: Give a better implementation for (*>) May 19, 2025
@L0neGamer
Copy link
Copy Markdown
Author

L0neGamer commented Jun 12, 2025

@ocharles if you don't mind, why is mzip and (<*>) different for TreeT?

I'm not sure why NodeT's applicative wasn't implemented like below. Could you shed some light?

diff --git a/hedgehog/src/Hedgehog/Internal/Tree.hs b/hedgehog/src/Hedgehog/Internal/Tree.hs
index 0970ec1..d95aa62 100644
--- a/hedgehog/src/Hedgehog/Internal/Tree.hs
+++ b/hedgehog/src/Hedgehog/Internal/Tree.hs
@@ -428,9 +428,9 @@ instance (Eq1 m, Eq a) => Eq (TreeT m a) where
 instance Applicative m => Applicative (NodeT m) where
   pure x =
     NodeT x []
-  (<*>) (NodeT ab tabs) na@(NodeT a tas) =
-    NodeT (ab a) $
-      map (<*> (fromNodeT na)) tabs ++ map (fmap ab) tas
+  (<*>) nf@(NodeT f tfs) na@(NodeT a tas) =
+    NodeT (f a) $
+      map (<*> fromNodeT na) tfs ++ map (fromNodeT nf <*>) tas
 
 instance Applicative m => Applicative (TreeT m) where
   pure =

@L0neGamer L0neGamer marked this pull request as draft October 17, 2025 09:00
@ocharles
Copy link
Copy Markdown
Contributor

ocharles commented Feb 4, 2026

Sorry, I don't know why they are different - I'm not familiar with the underlying code.

@moodmosaic
Copy link
Copy Markdown
Member

I think this makes (*>) behavior consistent with do notation, where you'd expect sequential execution.

@L0neGamer L0neGamer marked this pull request as ready for review February 5, 2026 20:01
@L0neGamer
Copy link
Copy Markdown
Author

I've changed this to instead just cleanup some instances and add a comment. I can adjust the changelog if wanted, but this seems minor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants