Implicits in for comprehensions/pattern matches SIP tracking

[dickwall]

After some initial activity, this one has been stationary since April.

https://github.com/scala/scala.github.com/pull/417

I contacted Harold twice over the last month but have received no reply yet.

Volunteer needed to track down Harold, and gauge current interest in pushing the SIP further along with status.

[dickwall]

Tweet from Harold indicates work is still in progress. Keeping open.

[SethTisue]

reference: https://twitter.com/haroldl/status/641017988322213888

[dickwall]

@haroldl any updates for the SLIP meeting tomorrow?

[som-snytt]

There's an alternative implementation (but not a SLIP) at https://github.com/scala/scala/pull/4730

[haroldl]

@som-snytt great looking code! Do you support if guards? I found that scenario very challenging to handle cleanly - see the "Challenges" section of the SLIP I wrote up.

[som-snytt]

@haroldl I don't like to toot my own horn, but there are some fine whitespace changes in that PR. It also reduces the debug noise when REPL starts.

I didn't quite follow your proposal. I can just about follow the current desugar until I lose the napkin with my notes.

The goal of this PR was just to attach IMPLICIT to the names, and make sure they're implicit when they pop out in the expansion. Nothing else changes.

scala> for (implicit b <- Some(true) if implicitly[Boolean]) println(implicitly[Boolean]) // show
object $read extends scala.AnyRef {
[snip]
      val res1 = Some(true).withFilter(((implicit b) => implicitly[Boolean])).foreach(((implicit b) => println(implicitly[Boolean])))
    }
  }
}
true

Similarly,

scala> for (implicit i <- 1 to 5 ; implicit s = "hi") yield (implicitly[Int], implicitly[String]) // show
[snip]
      val res2 = 1.to(5).map(((implicit i) => {
        implicit val s = "hi";
        scala.Tuple2(i, s)
      })).map(((x$1) => {
        <synthetic> <artifact> private[this] val x$2 = x$1: @scala.unchecked: @scala.unchecked match {
          case scala.Tuple2((i @ _), (s @ _)) => scala.Tuple2(i, s)
        };
        implicit val i = x$2._1;
        implicit val s = x$2._2;
        scala.Tuple2(implicitly[Int], implicitly[String])
      }))
    }

I see a bug that the i parameter to the map should be implicit; the modifier isn't propagated somewhere. (fixed)

This is an example of a PR where there's just no way a casual contributor will carry that napkin around long enough for it to get reviewed. For one thing, your pocket gets greasy from the napkin.

[haroldl]

Hi @som-snytt, I agree that the rules are complex enough as-is. That's why I abandoned my original approach. I think your code looks great. It would be good to cover the following case also:

def f(implicit x: Int, y: String) = x.toString == y

for {
  implicit a <- List(1, 2)
  implicit b <- Option("2")
  if f
} yield (a, b, f)

With your code, that for comprehension is de-sugared into the following:

List(1, 2).flatMap((implicit a =>
  Option("2").withFilter((b => f))
             .map((implicit b => scala.Tuple3(a, b, f)))))

Because the b parameter in the withFilter isn't marked implicit this won't compile. Maybe it will be a simple fix to propagate the implicit modifier into the filter on line 709 of your code.

[som-snytt]

@haroldl Yes, that works, see "fixed".

$ skala
Welcome to Scala 2.12.0 (Java HotSpot(TM) 64-Bit Server VM 1.8.0_60)

scala> def f(implicit x: Int, y: String) = x.toString == y
f: (implicit x: Int, implicit y: String)Boolean

scala> for (implicit a <- List(1, 2) ; implicit b <- Option("2") if f) yield (a, b, f) // show
[snip]
      val res0 = List(1, 2).flatMap(((implicit a) => Option("2").withFilter(((implicit b) => f)).map(((implicit b) => scala.Tuple3(a, b, f)))))
    }
  }
}
res0: List[(Int, String, Boolean)] = List((2,2,true))

Miles Sabin tweeted "no, thanks" like he was walking through the sample perfume gauntlet at a J.C. Penney. So who knows if this solution is worth even transient complexity.

[SethTisue]

I discussed this with @retronym just now. He's comfortable with it continuing to move forward.

@odersky's 2009 comment on SI-2823 ("welcome a [SIP] that contains spec language") seems to indicate that this SIP should be assigned a number and become active. So, I'll bring it up at the next SIP/SLIP meeting (attn @dickwall, keeper of the agenda) and let's make sure that 2015 Martin still feels the way 2009 Martin did. In 2015 we have Dotty to think about also, so that could be a factor.

(@adriaanm, want to weigh in before the meeting?)

@som-snytt, assuming the SIP becomes active, do you want to take ownership here and merge @haroldl's spec changes with your code changes? Martin indicated in 2009 that he considered the spec changes crucial here.

[som-snytt]

My specese would be minimal pidgin specese: use implicit at generator or midstream assignment, and the vars bound there remain implicit in all expansions.

That's Adriaan's comment on the proposal, and that's all I implemented.

When this came up a couple of years ago, I tried structural changes as in @haroldl proposal, for what seemed like intuitive scoping, etc; but I wouldn't be interested in trying that now, especially in the context of a trivial feature.

There may be issues like SI-9460 (multiple extractions in expansion) that might motivate more work. It depends on what the people who use for expressions say.

[dickwall]

SLIP committee thinks this should move forward. People can try out by adding sbt resolver for scala snapshots and put in the version number for the PR. Instructions will be posted below. @som-snytt do you have time to continue this work?

[dickwall]

Possible problem: In Dotty - want to enforce every implicit has explicit result type. How would that interact with these changes?

[som-snytt]

@dickwall Sure. Life is short, and soon I'll have all eternity to tinker with it. But srsly, it'll take the same dedication to bulletproof quality to decide if the impl is too flakey. The result type question is interesting. I think there's an issue hanging around somewhere to make the annotation on the generator act like a filter. for (i: Int <- values). IIRC.

[densh]

I think there's an issue hanging around somewhere to make the annotation on the generator act like a filter. for (i: Int <- values). IIRC.

You were thinking of SI-900.

[SethTisue]

People can try out by adding sbt resolver for scala snapshots and put in the version number for the PR. Instructions will be posted below

The last commit at https://github.com/scala/scala/pull/4730 is 889a68a, so according to the instructions from https://github.com/scala/scala/blob/2.12.x/README.md, the sbt commands to use the Scala built from that commit are:

set resolvers += "pr" at "https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots/"
set scalaVersion := "2.12.0-889a68a-SNAPSHOT"

this works for any PR, but I think many people don't know that, so it's worth reiterating.

[SethTisue]

SLIP committee thinks this should move forward

well yes, but with a pretty huge caveat: see meeting notes (https://github.com/scala/scala-lang/pull/358). @odersky seemed uninterested in having this go forward unless it's for patterns in general, not just for for comprehensions in particular. that's substantially more ambitious.

Martin points out that it Dotty already supports the more ambitious version as long as the types are annotated. so e.g. in Dotty this compiles: implicit val (a: Int, b: String) = (3, "foo")

[SethTisue]

P.S. in Dotty for (implicit a <- List(3, 4)) ... doesn't currently compile, but presumably making it compile wouldn't be a huge task in a Dotty context given that the pattern support is already there.

[som-snytt]

Normal implicit ValPats work, but the syntax to be invented is as @densh pointed out somewhere:

scala> Some((42, "hi")) match { case implicit Some((i: Int, s: String)) => implicitly[String] }

I think he put the implicit inside. Can I use sbt to test a build of dotty? @SethTisue

[SethTisue]

Can I use sbt to test a build of dotty?

if you mean, can I set scalaVersion := ... in my sbt project and it uses dotty instead, afaik the work to enable that it hasn't been done yet.

[SethTisue]

Dotty works with sbt now.

[SethTisue]

Closing since this repository is being decommissioned (#35).

This could still go forward under the current SIP process if someone steps forward to champion it.