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

for comprehension loses backticks when desugaring to val? #11533

Open
jducoeur opened this issue May 15, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@jducoeur
Copy link

commented May 15, 2019

This arose from this thread on Scala Users.

Take the following code (working fiddle, Scala 2.12):

val `P(X|C)`=3

println(`P(X|C)`)

val x = for {
  `F(Y|Z)` <- Option(3)
  `P(X|C)` <- Option(6)
  `A(B|C)` = 4  // fails here
}
  yield `P(X|C)`
  
println(x)

Everything compiles for me except the specified line, which is getting the error:

ScalaFiddle.scala:8: error: not found: value A(B|C)
    `A(B|C)` = 4
    ^

All of this suggests to me that something's a bit weird with backtick'ed identifiers when desugaring to a val. The desugars to flatMap and map appear to work, but the one to val doesn't...

@joroKr21

This comment has been minimized.

Copy link

commented May 15, 2019

Maybe it desugars to a pattern match? Typer says:

       private[this] val x: Option[Nothing] = scala.Option.apply[Int](3).flatMap[Nothing](((F(Y|Z): Int) => Option(6).map(((x$3: Int) => x$3: @scala.unchecked match {
  case (x$1 @ `P$u0028X$barC$u0029`) => {
    val <x$2: error>: <error> = 4: @scala.unchecked match {
      case (<x$2: error> @ `<A$u0028B$barC$u0029: error>`) => <x$2: error>
    };
    scala.Tuple2(x$1, <x$2: error>)
  }
})).map(((<x$4: error>: <error>) => <x$4: error>: @scala.unchecked match {
          case scala.Tuple2(`P$u0028X$barC$u0029`, `<A$u0028B$barC$u0029: error>`) => `P$u0028X$barC$u0029`
        }))));
@szeiger

This comment has been minimized.

Copy link

commented May 17, 2019

The spec is vague on this topic. The only thing I'm sure about is that something is wrong.

Starting with regular value definitions (https://www.scala-lang.org/files/archive/spec/2.12/04-basic-declarations-and-definitions.html#value-declarations-and-definitions). The formal syntax for the LHS is always at least a Pattern2, so the "simple name" case is not distinguished at the syntactic level. It is listed explicitly in the description:

Value definitions can alternatively have a pattern as left-hand side. If p is some pattern other than a simple name or a name followed by a colon and a type, then the value definition val p = e is expanded as follows

There is no formal definition of "simple name" in the spec. According to use of the term in earlier chapters it refers to an unqualified identifier which may also be "an arbitrary string between back-quotes".

The implementation of value definitions agrees with this. Any unqualified identifier, even when enclosed in backquotes, is treated as a simple name. No pattern matching applies.

According to https://www.scala-lang.org/files/archive/spec/2.12/06-expressions.html#for-comprehensions-and-for-loops:

A generator p <- e produces bindings from an expression e which is matched in some way against pattern p. A value definition p = e binds the value name p (or several names in a pattern p) to the result of evaluating the expression e

This could be interpreted to mean that p in a generator is always a pattern but that it can be a simple name in a value definition. Or it could be an imprecise way of saying that there are patterns that bind a single name p. My guess would be that it was originally intended to be the same as a normal val (remember, back in the day the syntax required a val keyword in this case) but maybe this was changed to align it with generators when the val keyword got removed. (Spoiler alert: The current implementation disagrees with both readings.)

This brings us to https://www.scala-lang.org/files/archive/spec/2.12/08-pattern-matching.html#stable-identifier-patterns:

To resolve the syntactic overlap with a variable pattern, a stable identifier pattern may not be a simple name starting with a lower-case letter. However, it is possible to enclose such a variable name in backquotes; then it is treated as a stable identifier pattern.

It does not say anything about other identifiers in backquotes. In practice, all identifiers in backquotes are interpreted as stable identifier patterns in proper patterns (i.e. not ones that are treated as simple names).

But since generators in for comprehensions are always patterns (according to the definition above), the first line (`F(Y|Z)` <- Option(3)) should not compile and the second line (`P(X|C)` <- Option(6)) should be a stable identifier pattern. So in practice generators do recognize simple names and treat them as simple definitions, not as pattern matches. This is consistent with value definitions. However, value definitions also contain an exception for what would otherwise be a typed pattern (val (x: String) = (1: Any), just like val x: String = (1: Any), is not a pattern match) but this is not mirrored by generators.

As the original bug report in this ticket shows, value definitions in for comprehensions are not consistent with either standalone value definitions or generators. They do not make the same exceptions for simple definitions. These are always treated as proper pattern matches.

By the way, when it comes to generators, there are other oddities. Take this simple typed pattern:

for((a: String) <- Option(1: Any)) yield a

It should desugar to:

Option(1: Any).withFilter { case (a: String) => true; case _ => false }.map { case (a: String) => a }

But actually it becomes

Option(1: Any).withFilter { case (a: String) => true; case _ => false }.map { (a: String) => a }

which doesn't compile. It looks like the compiler is isn't quite sure whether it's a typed assignment or a typed pattern.

@som-snytt

This comment has been minimized.

Copy link

commented May 17, 2019

They are all patterns. It seems to me the problem is with the other generators; the back-ticked pattern is refutable.

@szeiger

This comment has been minimized.

Copy link

commented May 17, 2019

Syntactically they are all patterns, but as I elaborated (far too long) above, that's not sufficient for deciding on their interpretation. Syntactically, val x = 1 is a pattern match. (x is parsed as a Pattern2), so is val (x: Int) = 1 ((x: Int) is a Pattern2), but curiously in val x: Int = 1 only x is parsed as a Pattern2. That's why you have to look to the explanations in the spec for the interpretation of certain simple patterns.

Refutability does not matter at that point. If you see `foo` you first have to decide if that means foo (simple name) or $x if $x == foo (stable identifier pattern).

@som-snytt

This comment has been minimized.

Copy link

commented May 17, 2019

I think you're overthinking it. The syntax to introduce a funky name is:

for (`funky name` @ _ <- List(27))

which corresponds to this syntax which was enhanced in 2.12, because previously it had to be a varid:

27 match { case `funky name` @ _ => `funky name` }

Ordinary val def syntax to introduce a name is not a pattern, irrespective of the productions.

The text says the LHS can also be a pattern, etc. Of course this leads to differing expectations about what the x: String syntax does in a generator (filtering or throwing or compile error, I don't even know what it does without testing).

@martijnhoekstra

This comment has been minimized.

Copy link

commented May 17, 2019

@szeiger

This comment has been minimized.

Copy link

commented May 17, 2019

I think you're overthinking it

I prefer to call this "others are underspecifying it"

@som-snytt

This comment has been minimized.

Copy link

commented May 17, 2019

@martijnhoekstra Yes, IMHO, that is the bug because it should be taken as a pattern. But I wouldn't be averse to rewriting the rules, either, or clarifying them the other way.

The umbrella issue is #900 which reminds me, I have a PR to revive that helps with retronym's workaround, to use x @ p to force "take as a pattern".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.