Skip to content
This repository has been archived by the owner on Oct 4, 2020. It is now read-only.

Unrefined Eff type #20

Closed
wants to merge 3 commits into from
Closed

Unrefined Eff type #20

wants to merge 3 commits into from

Conversation

paf31
Copy link
Contributor

@paf31 paf31 commented May 6, 2017

I chose not to export constructors so that we can eventually swap around the dependencies (regular Eff implemented using unrefined Eff plus a phantom row) later, if this proves to be useful.

@paf31 paf31 requested review from garyb and hdgarrood May 6, 2017 19:37
@hdgarrood
Copy link
Contributor

I like the look of this. Will the eff optimisations fire on this type too?


-- | A variant of `Control.Monad.Eff.Eff` without the row of effects.
-- |
-- | `Unrefined.Eff a` is isomorphic to `exists eff. Refined.Eff a`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this say Refined.Eff eff a?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks.

derive newtype instance applyEff :: Apply Eff
derive newtype instance applicativeEff :: Applicative Eff
derive newtype instance bindEff :: Bind Eff
derive newtype instance monadEff :: Monad Eff
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we give this type a MonadEff instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced unrefined actually.

@paf31
Copy link
Contributor Author

paf31 commented May 6, 2017

Will the eff optimisations fire on this type too?

I'm not sure. Not directly, but if enough newtype wrappers get removed, it's possible.

Edit: no, it looks like we need compiler support since the dictionaries aren't getting inlined.

@MonoidMusician
Copy link

@garyb
Copy link
Member

garyb commented May 7, 2017

^ I was going to say, does this need to go in core, since there's already the "IO" stuff? I'm not sure calling it Eff is a great idea if we are going to have it here too.

@paf31
Copy link
Contributor Author

paf31 commented May 7, 2017

does this need to go in core

Well, I'd like to see the dependency inverted and Eff depend on Eff.Unrefined eventually. Also, being in core means it can be optimized by the compiler. Finally, I'd like a simpler Eff to be in core so that I can talk about it in the book before the refined Eff, which appears for the first time near the middle of the book.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants