Allow \" in single-double-quoted string interpolations

[eed3si9n]

Changing "Hello, \"World\"" to s"Hello, \"$who\"" no longer breaks.

Before this change, \" terminated single-quoted interpolated string
literals, now the string remains open. The scanner doesn't interpret
the escape sequence, string interpolators can do so (s and f do).

Breaking changes:

• raw"c:\" no longer compiles, it's now an unclosed string
• raw"c:\" // uh" used to evaluate to """c:\""", now it's """c:\" // uh"""

Fixes scala/bug#6476

[SethTisue]

This change should be relatively safe

I think we need to actually know what the safety level is. Either it's absolutely safe, or there are unsafe cases that need to be precisely characterized in order to know whether we're comfortable with advancing this.

[som-snytt]

Not sure if you reviewed https://github.com/scala/scala/pulls?q=is%3Apr+author%3Asom-snytt+6476+is%3Aclosed

especially #4308 has a dreary tail.

[eed3si9n]

@som-snytt I did see some of the discussion that took place, and that's one of my motivation behind lawyering up on the legality within current statute.

[eed3si9n]

I cherry picked 195d369 from Som's 2015 PR. This handles last backslash, so it's better than my implementation.

[OlegYch]

all escapes should be treated in the same way, not just \"

[eed3si9n]

@SethTisue Let's define safety as non-existence of:

• semantic changes to previously-compiled binaries
• semantic changes to previously-admissible sources
• admittance of illegal program

We can rule out first and last I think. Is there a source that we can write that would change the meaning? raw-interpolator captures the character sequence, so if there's a problem, we should see it using raw.

• My first commit didn't handle backslash in last position raw"foo\". That's now fixed with Som's change.
• That in turn introduced \$ changing the semantics of raw"\$foo". I rolled that back.

However I can make up a program that would evaluate differently:

println(raw"1\"); println(2); // \"")

In Scala 2.13.1 this would print:

scala> println(raw"1\"); println(2); // \"")
1\
2

With this patch:

1\"); println(2); // \"

It would be very unlikely that you'd have a comment // \"") lying around there so it would more likely fail to compile.

[som-snytt]

I also noticed that looking ahead for another quote doesn't handle comments; it's a pretty severe limitation, but only worth fixing if the PR has a chance.

[eed3si9n]

@OlegYch

all escapes should be treated in the same way, not just "

The other backslash escape sequences like \n already work with s"...". It's just that \" currently is treated as the-end-of-literal by the scanner, which this PR aims to fix. Do you have specific example that I missed?

[OlegYch]

@eed3si9n sorry i thought this was related to raw strings, nvm then

[dwijnand]

Some comments from reviewing.

[dwijnand]

Also, we should fix the REPL, as it behaves differently to real compilation:

scala> raw"\\"
val res0: String = \
scala> raw"\"
val res1: String = ""
scala> "\""
     |
     |
You typed two blank lines.  Starting a new command.
scala> "\\"
       ^
       error: unclosed string literal

[lrytz]

It would be very unlikely that you'd have a comment // \"") lying around there so it would more likely fail to compile.

Dale and me were trying to come up with a few more examples around this; so your point is that the change can only prevent code from compiling that didn't work before, or vice-versa, but not make compile and run code differently (except for the very corner case you highlight)?

this no longer compiles

$> spr 8830 -e 'print(raw"\"); println("")'
/var/folders/py/8q6hn5qj3g365g0tsy63f68c0000gn/T/scalacmd9448771494255440057.scala:1: error: unclosed string literal
print(raw"\"); println("")
                        ^

this now compiles

$> spr 8830 -e 'print(raw"\"); println(\"")'
\"); println(\"

I personally have doubts about speccing the isLastQuoteOnLine part. I think it's preferrable to go with the breaking change and treat \" in an interpolated string as not closing the string.

[martijnhoekstra]

$" still makes more sense to me than \" as escaped quote: it breaks nothing, and it's more regular in that $ is the meta-character for interpolations, while how \ is treated is up to the interpolator itself: that it's treated the same as in regular strings in the s and f interpolator is a property of those interpolators, not of interpolations in general.

[lrytz]

I agree with that in principle, and I think we should have the fix you did in Scala 3 also in Scala 2.

However, I really think for user-friendlyness moving from "he said \"hi\"" to s"$he said \"hi\"" should work. If it can be done without breaking too much.

[dwijnand]

$" still makes more sense to me than \" as escaped quote

A subtlety (perhaps you knew this already): this doesn't treat \" as an escaped quote, it treating it as not the terminator of the string. Compare how raw keeps \" but doesn't keep $":

$ scala --scala-pr 8830 -e 'println(s"he said: \"hi\" to me")'
he said: "hi" to me
$ scala --scala-pr 8830 -e 'println(raw"he said: \"hi\" to me")'
he said: \"hi\" to me
$
$ scala3
Starting scala3 REPL...
scala> s"he said: $"hi$" to me"
val res0: String = he said: "hi" to me

scala> raw"he said: $"hi$" to me"
val res1: String = he said: "hi" to me

[eed3si9n]

I personally have doubts about speccing the isLastQuoteOnLine part. I think it's preferable to go with the breaking change and treat \" in an interpolated string as not closing the string.

If I remember correctly, I cherry picked the last-\ support from a Som PR, so if we go with just the first commit in this PR that's likely what you want.

[som-snytt]

#4308 (comment) is still true.

The old PR is still an easy rebase on 2.12:

scala> raw"C:\Users\snytt\"
res4: String = C:\Users\snytt\

scala> raw"C:\Users\snytt\"    with a stray trailing quote    "
res5: String = "C:\Users\snytt\"    with a stray trailing quote    "

People misplace quotes and braces all the time and experience brain failure until they figure it out.

A close quote is the next unescaped quote, or the last escaped quote on the line.

Many of the "what about this" examples don't pass the Odersky test of sane people don't code like that.

[lrytz]

(t7020.scala is flaky currently, likely since #9522)

[lrytz]

Pushed some spec'ese words for this change.

[lrytz]

Rebased and squashed, updated the PR description. This is now ready, but let's see what the Scala 3 people say over at scala/scala3#11751.

[dwijnand]

This is now ready, but let's see what the Scala 3 people say over at lampepfl/dotty#11751.

That was merged. So shall we (f-i-nally!) merge this now?

[SethTisue]

@som-snytt you'll have to stop saying "when string interpolators support escaped double quotes" and go back to saying "when hell freezes over" like before

@eed3si9n thanks for not giving up on this :-)

[eed3si9n]

Never gonna give \" up, never gonna let s"\"" down. I'm happy we found a way to fix this in both Scala 2 and 3.

[som-snytt]

I was going to go with flying pigs. My favorite part re-reading the ticket is when SethTisue says this is impossible now and eed3si9n submits a PR.

[SethTisue]

My favorite part re-reading the ticket is when SethTisue says this is impossible now and eed3si9n submits a PR.

Can't wait to find out who plays me in the movie version.