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

[WIP] feat(std): replace parser lib with more generic version #687

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Etherian
Copy link
Contributor

@Etherian Etherian commented Feb 12, 2019

Description

Adds a Parsable interface and a parser combinator library based on it to replace the current one.

Related PRs

Depends on #686.

Similar concept to the Streamlike interface (#737)

Status

In Development

Outstanding design questions:

  • How should parser handle errors and custom error types? (It will use its own error type. No customization for now.)
  • How to cleanly handle types that don't naturally lend themselves to StateT sequencing (String and Array)? (Most likely a wrapper, like in the current parser.)

To Do

  • Get StateT to a run without errors to begin debugging.
  • Add useful functions
  • Add implementations for standard types
  • Add inline documentation
  • Add documentation in the book?

Prior art

Future Possibilities

  • Build Parsable on top of Streamlike or otherwise take advantage of their relatedness.
  • Indexable -> Parsable?

@Etherian Etherian changed the title feat(std): replace parser lib with more generic version [WIP] feat(std): replace parser lib with more generic version Feb 12, 2019
Copy link
Member

@Marwes Marwes left a comment

Choose a reason for hiding this comment

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

I like the basic plan of adding a StreamLike type which Parser can be abstracted over but I think it may be a good a idea to take a step back and go for something a bit simpler first (see other comments). The ZipWith, Lazy etc abstractions over StreamLike are neat, but aren't strictly necessary to for the basic idea to work so I think forcusing on just getting Parser working with StreamLike first will make it easier (and then the rest can be added later).

(Seems I forgot/failed to press submit review on this, sorry for the delay!)

std/parser.glu Outdated Show resolved Hide resolved
let string @ { String, ? } = import! std.string

// TODO How handle atom streams generically? Stream-like interface?
// TODO How handle error position generically? Zip stream with enumeration?
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to represent a position as an associated type however that does not work in gluon (yet). Could maybe parameterize StreamLike with a position argument though.

std/parser.glu Outdated Show resolved Hide resolved
std/streamlike.glu Outdated Show resolved Hide resolved
std/streamlike.glu Outdated Show resolved Hide resolved
std/streamlike.glu Outdated Show resolved Hide resolved
std/streamlike.glu Outdated Show resolved Hide resolved
std/streamlike.glu Outdated Show resolved Hide resolved
bors bot added a commit that referenced this pull request May 4, 2019
686: [WIP] feat(std): add monad transformer interface, StateT, and LazyT r=Marwes a=Etherian

## Description
Adds a monad-transformer interface `Transformer`, a State monad transformer `StateT`, and a Lazy monad transformer `LazyT` to the standard library.

## Related PRs
Lays groundwork for Streamlike and the new parser #687.

## Status
**In Development**

### Outstanding design questions
- [x] ~~Can and should `StateT` be replaced by the effects system?~~ ([not for the time being, at least](#686 (comment)))
- [ ] Should `Transformer` require `monad` field?
- [ ] Should StateT and LazyT (and other monad transformers) be merged with their non-transformer counterparts?

### To Do
- [x] ~~Fix StateT's implicit parameter bugs~~ (#688 & #689)
- [ ] add tests
- [ ] add inline docs

## Prior Art
- StateT
  - [Haskell](https://hackage.haskell.org/package/transformers-0.5.6.2/docs/src/Control.Monad.Trans.State.Lazy.html#StateT)
  - [Idris](https://github.com/idris-lang/Idris-dev/blob/master/libs/base/Control/Monad/State.idr)
  - [Implementing Applicative (<*>) for StateT](https://stackoverflow.com/questions/27903650/implementing-applicative-for-statet)
- MonadTrans
  -  [Haskell](https://hackage.haskell.org/package/transformers/docs/src/Control.Monad.Trans.Class.html#MonadTrans)
  - [Idris](https://github.com/idris-lang/Idris-dev/blob/master/libs/base/Control/Monad/Trans.idr)

Co-authored-by: Etherian <[email protected]>
Co-authored-by: Markus Westerlind <[email protected]>
@Etherian
Copy link
Contributor Author

Ignore std/parser.glu for now. It's still a mess.

But what do you think of the Alternative implementation for Result? I think it makes sense.

@Marwes
Copy link
Member

Marwes commented May 25, 2019

But what do you think of the Alternative implementation for Result? I think it makes sense.

It doesn't seem to compile as is :( . Searched for what Haskell does with its equivalent Either type and there doesn't seem to be a way to construct such as an instance https://stackoverflow.com/questions/44472008/why-is-there-no-alternative-instance-for-either-but-a-semigroup-that-behaves-sim

@Etherian
Copy link
Contributor Author

Etherian commented May 26, 2019

Sorry. It didn't compile because I wrote it wrong the first time. Alternative (Result (m e)) seems to work now.

If I understand correctly, the answerer of that question says that Alternative can't be implemented for Either because Haskell doesn't allow you to add constraints to the instantiation that don't exist in the definition (maybe that requires an extension?).

While it is possible to construct a type where e has an instance of Alternative you can't make an instance for Alternative for Either with that e because no such constraint is in the type class definition (something along the lines of: (Alternative e, Applicative (f e)) => Alternative (f e)).

But doing so works just fine in Gluon.

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.

2 participants