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

SIP-??? created pending: Implicits in for comprehensions #417

Closed
wants to merge 3 commits into from

Conversation

haroldl
Copy link

@haroldl haroldl commented Apr 11, 2015

I'd like to propose SIP-24: Implicits in for comprehensions.

@haroldl
Copy link
Author

haroldl commented Apr 14, 2015

Thank you both for taking a look at this!

I'm not sure how this is making Option special. I'm generating AST that maps to code that wraps a value in Some, or else returns None, and then flattening it away so the Option[T] values never reach the outside code.

I'm not actually using flatten, I'm translating it to ".flatMap { x => x }" where x is fresh in order to avoid adding any requirements on the source data types for generators. Today they are only required to provide flatMap, map and withFilter and I didn't want to change that.

My next steps are to read the quasiquotes and comprehensive comprehensions docs mentioned by @retronym and figure out how that impacts my work and try making the changes suggested by @adriaanm. This may take me a few days.

I've also updated the code:

I've squashed my commits and moved them to a new branch sip24a based on 2.12.x to be more modern (and added some more tests).

I've changed the nme.Some / nme.None to use definitions.SomeModule / definitions.NoneModule to avoid picking up shadowing definitions of Some / None.

@retronym
Copy link
Member

I'm generating AST that maps to code that wraps a value in Some, or else returns None, and then flattening it away so the Option[T] values never reach the outside code.

This depends on the flatMap method supporting return values of type Option[_]. That holds for the collections (given the impicit Option.optionToIterable), but doesn't generalize to other monadic types:

trait M[A] {
  def map[B](a: A => B): M[B]
  def flatMap[B](a: A => M[B]): M[B]
  def filter(f: A => Boolean): M[A]
}

object Test {
  def guard = true
  def testOk  (m: M[String]) = m.filter(_ => guard).map(x => x)
  def testFail(m: M[String]) = m.flatMap(x => if (guard) Some(x) else None)
}
qscalac sandbox/test.scala
sandbox/test.scala:10: error: type mismatch;
 found   : Some[String]
 required: M[?]
  def testFail(m: M[String]) = m.flatMap(x => if (guard) Some(x) else None)
                                                             ^
sandbox/test.scala:10: error: type mismatch;
 found   : None.type
 required: M[?]
  def testFail(m: M[String]) = m.flatMap(x => if (guard) Some(x) else None)
                                                                      ^
two errors found

The current encoding using filter/withFilter only requires a common understanding of Boolean.

@haroldl
Copy link
Author

haroldl commented Apr 14, 2015

If people are okay with broadening the scope per the suggestion from @densh then the desugaring rules become much simpler and the Option + flatMap(x => x) piece just goes away.

Otherwise, I'll look for alternatives that do not depend on Option though there may need to be some other compromise such that the carrier type needs to implement something else in addition to what is currently required (which would be some method/feature of the existing collections) in order to support implicits in for comprehensions.

@retronym
Copy link
Member

+1 to the generalization

@SethTisue SethTisue changed the title SIP-24 created pending: Implicits in for comprehensions SIP-??? created pending: Implicits in for comprehensions Aug 23, 2015
@dickwall
Copy link
Contributor

dickwall commented Sep 7, 2015

Have emailed and tweeted Harold to glean continuing interest in pursuing this. Will likely move to dormant in the next SIP/SLIP committee meeting if nothing heard back.

@haroldl haroldl closed this Nov 10, 2015
@haroldl
Copy link
Author

haroldl commented Nov 10, 2015

I've closed this PR since I haven't found time to pursue the generalization suggested by @retronym and I think that that is the right way to proceed.

@Jacke
Copy link

Jacke commented Mar 5, 2020

FYI nowadays you can get the same functionality by using https://github.com/oleg-py/better-monadic-for#define-implicits-in-for-comprehensions-or-matches

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.

None yet

6 participants