feat(Control.Monad.Writer.CPS): re export runWriterT #136
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Why is this modification necessary? Writer.Strict, the transformers exposes the internal field of WriterT, which is runWriterT. However, for Control.Monad.Trans.Writer.CPS, it does not expose the internal fields and is a separate function, so it needs to be added to the import/export list.
The same is probably necessary for writerT since the inside was covered up as well. I gave up on writer because the name was covered.
|
It was probably overzealous re-export pruning done during the PVP compliance arc for this package. Re-exporting is fine. |
emilypi
approved these changes
Dec 8, 2022
ncaq
added a commit
to ncaq/logict
that referenced
this pull request
Dec 23, 2022
I have implemented `MonadLogic` in `WriterT` as well. Until now, `WriterT` should not be used because it easily causes space leaks. However, [Control.Monad.Trans.Writer.CPS](https://www.stackage.org/haddock/nightly-2022-12-08/transformers-0.5.6.2/Control-Monad-Trans-Writer-CPS.html) no longer causes space leaks and can be used. Therefore, we would like to use `WriterT`, but it is inconvenient if it cannot be combined with `MonadLogic`. Therefore, I implemented it and added tests where `ReaderT` and `StateT` are tested. To use the CPS version of `WriterT`, I raised the mtl version requirement to [feat(Control.Monad.Writer.CPS): re export runWriterT by ncaq · Pull Request #136 · haskell/mtl](haskell/mtl#136) has not yet been imported, so we have no choice but to rely directly on transformers. In the end, mtl depends on transformers, so we have determined that this is not a critical problem. We have taken care to make it easy to remove them when they are no longer needed by doing a limited `import`. I was surprised myself that many of the CPP macros in the test code were removed. It was not my intention. I believe it was probably done by the HLS plugin. I've been trying to find out why it deleted them. I found out that the new mtl raises the lower limit for the base library in the cabal file. So when raising the lower version limit of mtl, we thought that it is certainly not a very effective practice to control the base version here. Therefore, we decided to leave the deleted part as it is.
ncaq
added a commit
to ncaq/logict
that referenced
this pull request
Jan 18, 2023
I have implemented `MonadLogic` in `WriterT` as well. Until now, `WriterT` should not be used because it easily causes space leaks. However, [Control.Monad.Trans.Writer.CPS](https://www.stackage.org/haddock/nightly-2022-12-08/transformers-0.5.6.2/Control-Monad-Trans-Writer-CPS.html) no longer causes space leaks and can be used. Therefore, we would like to use `WriterT`, but it is inconvenient if it cannot be combined with `MonadLogic`. Therefore, I implemented it and added tests where `ReaderT` and `StateT` are tested. To use the CPS version of `WriterT`, I raised the mtl version requirement to [feat(Control.Monad.Writer.CPS): re export runWriterT by ncaq · Pull Request #136 · haskell/mtl](haskell/mtl#136) has not yet been imported, so we have no choice but to rely directly on transformers. In the end, mtl depends on transformers, so we have determined that this is not a critical problem. We have taken care to make it easy to remove them when they are no longer needed by doing a limited `import`.
Bodigrim
pushed a commit
to Bodigrim/logict
that referenced
this pull request
Jan 18, 2023
I have implemented `MonadLogic` in `WriterT` as well. Until now, `WriterT` should not be used because it easily causes space leaks. However, [Control.Monad.Trans.Writer.CPS](https://www.stackage.org/haddock/nightly-2022-12-08/transformers-0.5.6.2/Control-Monad-Trans-Writer-CPS.html) no longer causes space leaks and can be used. Therefore, we would like to use `WriterT`, but it is inconvenient if it cannot be combined with `MonadLogic`. Therefore, I implemented it and added tests where `ReaderT` and `StateT` are tested. To use the CPS version of `WriterT`, I raised the mtl version requirement to [feat(Control.Monad.Writer.CPS): re export runWriterT by ncaq · Pull Request #136 · haskell/mtl](haskell/mtl#136) has not yet been imported, so we have no choice but to rely directly on transformers. In the end, mtl depends on transformers, so we have determined that this is not a critical problem. We have taken care to make it easy to remove them when they are no longer needed by doing a limited `import`.
|
👋 any reason this has not been merged yet? |
|
@kozross mind +1 'ing? |
|
I just tripped over this also. Would be great to see it merged. |
|
@kozross +1'ing again |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why is this modification necessary?
Writer.Strict, the transformers exposes the internal field of WriterT, which is runWriterT. However, for Control.Monad.Trans.Writer.CPS, it does not expose the internal fields and is a separate function, so it needs to be added to the import/export list.