Upgrade f-interpolator

[som-snytt]

Fixes #9939
Fixes #11256
Fixes #11750
Fixes #13293

[anatoliykmetyuk]

@dos65 @som-snytt any news on this one?

[som-snytt]

It's not pretty but is mergeable. Future improvements would include optimizing and constant folding.

I can give it my eyes one more time this week.

[anatoliykmetyuk]

So you're still planning to do some work on it? Or can @dos65 review it?

[som-snytt]

I'll give it a quick pre-review and click ready for review.

(Edit: removed observation that any LOC is burdensome.)

[dos65]

@som-snytt Nice work! Glad to see that the parity with Scala2 is finally obtained.

I left some comments/suggestions

[som-snytt]

Sorry I was busy this past week, I meant to update this over the weekend! Still plan to look at this next time I touch the keyboard. Thanks for giving it eyes!

[som-snytt]

I guess the best part is 850 lines added, 851 lines removed.

Edit: the other best part was figuring out again how to run a test.

[anatoliykmetyuk]

@dos65 @som-snytt what's the status on this one? Is the PR review-ready in which case WDYT about it @dos65 ?

[dos65]

@anatoliykmetyuk I think it's almost ready to merge. The only thing I'm worried that the FormatChecker implementation might be simplified if it conversions would be based on Type instead of ClassTag + from general approach I see that ClassTag isn't used in this way in scala3-compiler.

@som-snytt Don't you mind if I'll try to address this thing and will do a PR onto your branch? I think I will have time for that on this week.

[som-snytt]

My previous comment on use of ClassTag still seems true to me:

Actually, I want to backport to Scala 2, so a goal was for some target independence. That's useful for any future fixes or improvements.

ClassTag sufficed. I think I started by abstracting over Type but that was baroque.

I didn't follow up on ensuring s"hello, world" is a constant string, or s"hello, ${ "world" }" etc:

I'll take another look at the class structure. I remember an open issue was optimization or at least constant folding; not sure if that is better solved here or generally. Other optimizations would be along the lines of "replace %d with .toInt" or whatever.

[dos65]

@som-snytt

Actually, I want to backport to Scala 2, so a goal was for some target independence.

What do you mean by backport to Scala2? Do you want to port it back? I thought that this PR is a port from Scala2 🤔

I didn't follow up on ensuring s"hello, world" is a constant string, or s"hello, ${ "world" }" etc:

I've checked this sample, it's constant. Is there anything else to check?

I think there is a need to ask someone from core compiler team for a final review as it's a large PR.
From my side, I see that it works well, implementation is close to scala2-compiler, all tests from Scala2 were ported and pass 🎉

[som-snytt]

yes, I intended to backport the forwardport. 😄

I won't stand in the way of whatever you'd like to improve, for some definition of improve.

Mostly I feel that the compiler should not ingest all this code that ought to be library code, but the first step is just correctness. There is no binary compat constraint so it can be fixed/improved at any time.

[som-snytt]

While I think of it, config.Settings is an example use of ClassTag to represent erased types in compiler.

[som-snytt]

Embraced @dos65 's suggestions and dropped some code. Although I embraced the suggestions, I dropped some braces. I remember how nice it was to have the braces removed from my teeth in middle school.

[som-snytt]

Improved test output to show where // error is extra or missing. A test had 50 errors, impossible to correlate while sober.

[anatoliykmetyuk]

@som-snytt please let us know once this is review ready :)

[som-snytt]

Definitely ready. I could definitely make improvements indefinitely.

[som-snytt]

Rebased on phase name conflict.

Tweaked the f-usages that required backslashes to obviate returning to it later.

An unbootstrapped compiler will show doubled backslash in a few messages:

<   |              octal escape literals are unsupported: use \u0000 instead
---
>   |              octal escape literals are unsupported: use \\u0000 instead

"Caveat backslasher."

[anatoliykmetyuk]

LGTM🎉
Thanks @som-snytt for all the work, and @dos65 for reviewing it!

[mbovel]

Non-bootstrapped tests are failing. See logs.

[mbovel]

Could reproduce locally (sbt testCompilation t7292-removal):

-- Error: tests/neg/t7292-removal.scala:2:14 ---------------------------------------------------------------------------
2 |  val chr1 = '\0' // error
              ^
              octal escape literals are unsupported: use \\u0000 instead
-- Error: tests/neg/t7292-removal.scala:3:17 ---------------------------------------------------------------------------
3 |  val str1 = "abc\123456" // error
                 ^
                 octal escape literals are unsupported: use \\u0053 instead
-- Error: tests/neg/t7292-removal.scala:4:12 ---------------------------------------------------------------------------
4 |  val lf = '\012' // error
            ^
            octal escape literals are unsupported: use \n instead

The problem seems to be the duplicated slash in \\u.

[som-snytt]

See my last comment. I wasn't sure about the protocol. On Scala 2, the change would wait until "restarr". Should I submit a revert for that commit? FSR, it wasn't clear to me that tests were run unbootstrapped.