-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inject/Project find effects nested on the left #219
Conversation
eff (R (R (R other))) = CullC (eff (R (R (R (handleCoercible other))))) | ||
eff (R (L (L Empty))) = empty | ||
eff (R (L (R (Choose k)))) = k True <|> k False | ||
eff (R (R other)) = CullC (eff (R (R (handleCoercible other)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically just reassociating here.
inj = reassoc . inj where | ||
reassoc (L l) = L (L l) | ||
reassoc (R (L l)) = L (R l) | ||
reassoc (R (R r)) = R r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reassociation here won’t be too bad a performance hit as long as we’re reasonable about using left-nested sums.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on what you mean by this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can we document why we need to reassociate here (and maybe drop a link to DTALC for people who are wondering about why all these OVERLAPPABLE pragmas need to happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on what you mean by this?
Basically, this assumes that trees will generally be right-leaning. So anything exclusively right-chained won’t ever see this instance, and if you have (A :+: B) :+: C
that’s an extremely minor hit (as your benchmark demonstrates). But a long left-chain—which is extremely unlikely to happen, and never by accident; you would have to work quite hard to design your effects thus—will take more of a hit because it has to do a bunch more reassociation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can we document why we need to reassociate here (and maybe drop a link to DTALC for people who are wondering about why all these OVERLAPPABLE pragmas need to happen?
I’m gonna do this in #223 to avoid conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some documentation quibbles, and I confess that I’m not looking forward to changing these reassociations in semantic
, but this is the right thing, I think.
inj = reassoc . inj where | ||
reassoc (L l) = L (L l) | ||
reassoc (R (L l)) = L (R l) | ||
reassoc (R (R r)) = R r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on what you mean by this?
inj = reassoc . inj where | ||
reassoc (L l) = L (L l) | ||
reassoc (R (L l)) = L (R l) | ||
reassoc (R (R r)) = R r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can we document why we need to reassociate here (and maybe drop a link to DTALC for people who are wondering about why all these OVERLAPPABLE pragmas need to happen?
I can’t think of anything in |
This PR allows effects to occur on the left as well as the right, which will enable us to split
Error
up into bothThrow
andCatch
, reintroduceNonDet
as a synonym forEmpty :+: Choose
, etc.NonDet
as a synonym forEmpty :+: Choose
.NonDet
in theCarrier
instances forNonDetC
,CutC
, andCullC
.SplitsDeferring to a future PR.Error
intoThrow
&Catch
.RedefinesDeferring to a future PR.Fail
as a synonym forThrow String
.