🔥 interposition #223
🔥 interposition #223
Conversation
| eff (op :: sig (InterposeC eff m) a) | ||
| | Just (op' :: eff (InterposeC eff m) a) <- prj op = do | ||
| handler <- InterposeC ask | ||
| lift (runHandler handler (handleCoercible op')) |
There was a problem hiding this comment.
This removal is the essence of this PR. The justification is pretty straightforward:
-
It’s asymptotically inefficient.
prjoperates in linear time, in contrast with pattern matching on the outermost layer of the sum in constant time (the normaleffidiom). -
It’s pragmatically inefficient. Passing handlers around as higher-order functions doesn’t enjoy fusion; cf
InterpretCfor the reflection trick used to work around that. -
It’s equivalent to the much more efficient idiom of using another carrier (e.g.
InterpretC) to handle (another instance of) the effect at the outside of the sum. E.g. if you want to interpose between requests for some underlying effectE, then simply add another carrier for it at the outside, and you will get exactly the desired behaviour, complete with the ability to forward requests on to the underlying carrier (eavesdropping) if so desired.
|
|
||
|
|
||
| class Project (sub :: (* -> *) -> (* -> *)) sup where | ||
| prj :: sup m a -> Maybe (sub m a) |
There was a problem hiding this comment.
InterposeC represented the only use of Project in the library, so I’ve opted to remove it. The justification for this is a little bit weaker, IMO, but in short: it’s impolite, and probably surprising, to (metaphorically) wander into other people’s houses uninvited.
We currently use prj in three places in semantic (all of which represent some variation on interposition which should be changed), as well as several places in semantic-core and path (all of which represent syntactic operations which we could support by redefining Project/prj locally).
The latter category make me wonder if we should retain this for syntax use cases, but tbh bound-style syntax having the same kind as higher-order effects is at best a pun, because the syntax is generally a very different shape:
- Nullary constructors do not represent abnormal termination of control.
- Constructors are designed to hold terms, rather than continuations per se.
- One primarily wants to pattern match on syntax in a rather ad hoc fashion, rather than exclusively eliminating it via folds.
This makes Carrier instances beyond the typical Term at best inconvenient, and perhaps warrants a fused-syntax package enabling e.g. algebras for non-Monadic carriers. (And perhaps that package and fused-effects will someday share some higher-order syntax such as sums. Life is full of possibility.)
Thus, while I’m slightly concerned about this, I am currently, on the balance, convinced that removing it is right for the library; but I would nevertheless particularly welcome a gut check.
| @@ -62,30 +60,6 @@ instance (Carrier sig m, Effect sig) => Carrier (Reader r :+: sig) (ReinterpretR | |||
| eff (R other) = ReinterpretReaderC (eff (R (handleCoercible other))) | |||
|
|
|||
|
|
|||
| interposition :: Spec | |||
| interposition = describe "interposition" $ do | |||
There was a problem hiding this comment.
tbh we could have got rid of this when we added InterposeC.
| ( -- * Membership | ||
| Member(..) | ||
| -- * Sums | ||
| , (:+:)(..) |
There was a problem hiding this comment.
The docs read a lot better with Member listed first, due to the extremely large number of instances involving :+:.
| -- | ||
| -- This is based on Wouter Swierstra’s design described in [Data types à la carte](http://www.cs.ru.nl/~W.Swierstra/Publications/DataTypesALaCarte.pdf). As described therein, overlapping instances are required in order to distinguish e.g. left-occurrence from right-recursion. | ||
| -- | ||
| -- It should not generally be necessary for you to define new 'Member' instances, but these are not specifically prohibited if you wish to get creative. |
There was a problem hiding this comment.
Per #219 (comment), I’ve documented the overall design, the overlapping instances, and the reassociation; I’ve also noted here that you probably don’t need to define your own instances.
There was a problem hiding this comment.
My take on this is that it definitely improves the library, and since we have another answer for the use-case that Interpose addresses, it’s a good thing to kill prj. However, I don’t think we should merge this until we fix github/semantic#289, because the use of Interpose in Semantic is (surprise, surprise) very subtle, and I don’t want to get it wrong when we port semantic to fused-effects-1.0. Does that sound good?
IMO we can just copy in |
|
We’re gonna go ahead with the merge 👍 |
This PR:
Project.InjecttoMember.Memberinstances, per Inject/Project find effects nested on the left #219 (comment).