Skip to content
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

Flip arguments of run* eval* and exec* functions #95

Merged
merged 4 commits into from
Apr 20, 2018

Conversation

samuelpilz
Copy link
Contributor

resolves #94.
This flips arguments of run*, eval* and exec* functions of State, Reader and Writer. The other effects are left untouched intentionally. Mainly because I understand them not well enough.
There are also new eval*Writer functions that were "missing".

Copy link
Owner

@suhailshergill suhailshergill left a comment

Choose a reason for hiding this comment

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

fix build errors

runStateR :: Eff (Writer s ': Reader s ': r) w -> s -> Eff r (w,s)
runStateR m0 s0 = loop s0 m0
runStateR :: s -> Eff (Writer s ': Reader s ': r) w -> Eff r (w,s)
runStateR s m = loop s m
Copy link
Owner

@suhailshergill suhailshergill Apr 18, 2018

Choose a reason for hiding this comment

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

@power-fungus please fix the name shadowing error. i believe this is triggering it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be indeed the issue. However, using stack build this does not get detected. Is there a way to reduce the complexity of the Makefile by using stack? stack builds is fine, but does not report the shadowing errors. I don't understand the Makefile well enough to transform that. make build fails on my machine with

cabal sandbox init
Writing a default package environment file to
/home/power-fungus/Documents/haskell/extensible-effects/cabal.sandbox.config
Using an existing sandbox located at
/home/power-fungus/Documents/haskell/extensible-effects/.cabal-sandbox
cabal clean
cleaning...
cabal install --only-dependencies --enable-tests --enable-benchmarks
Warning: The package list for 'hackage.haskell.org' does not exist. Run 'cabal
update' to download it.
Resolving dependencies...
cabal: Could not resolve dependencies:
rejecting: extensible-effects-2.5.2.0:!bench (constraint from config file,
command line flag, or user target requires opposite flag selection)
trying: extensible-effects-2.5.2.0:*bench
unknown package: test-framework-th (dependency of
extensible-effects-2.5.2.0:*bench)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: extensible-effects,
extensible-effects-2.5.2.0:bench, test-framework-th
Note: when using a sandbox, all packages are required to have consistent
dependencies. Try reinstalling/unregistering the offending packages or
recreating the sandbox.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, it is possible to travis-cache the dependencies (which I know how to do for stack) but not for cabal with sandboxes

Copy link
Owner

Choose a reason for hiding this comment

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

@power-fungus a couple of points:

  • make build will fail unless a particular version of cabal and ghc are in scope. when developing, i bring those in via nix by running nix-shell (or a variant)
  • happy to move make build steps to use stack instead. patches welcome! (same for travis-cache issue)

{-# INLINE execWriter #-}

-- | Handle Writer requests, using a List to accumulate values and discard the
-- effect result.
Copy link
Owner

Choose a reason for hiding this comment

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

@power-fungus minor nitpick, but could we make the documentation consistent with execState etc? e.g. focusing on how the monoidal value is handled (instead of the result of computation). something like Handle Writer requests, using a List to accumulate values and returning the final accumulated values

if you agree this helps, could you tweak? if you don't, please ignore this

Copy link
Contributor Author

@samuelpilz samuelpilz Apr 19, 2018

Choose a reason for hiding this comment

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

good point. I did not think of that wording.

@samuelpilz
Copy link
Contributor Author

I will also address the next compilation errors in benchmarks

@suhailshergill suhailshergill merged commit f40abf5 into suhailshergill:develop Apr 20, 2018
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.

Argument ordering of run* functions
2 participants