SI-8650 F-interpolator inlines literals by som-snytt · Pull Request #4316 · scala/scala

som-snytt · 2015-02-11T00:55:21Z

FormatInterpolator emits a call to String.format(s, args...) and inlines
constant expressions in the string.

Indexed args are verified for correctness.

scala> final val pi = 3.14
pi: Double(3.14) = 3.14

scala> f"$pi%.1f %<s %1$$e"                    // just a string
res0: String = 3.1 3.14 3.140000e+00

scala> import System.{ currentTimeMillis => now }
import System.{currentTimeMillis=>now}

scala> f"$now%d"
res1: String = 1423615388758

scala> f"$now%tdth day of %<th"
res2: String = 10th day of Feb

scala> f"Today is the %1$$tdth day of %<th, epoch time $now%tQ"
res3: String = Today is the 10th day of Feb, epoch time 1423616802039

scala> f"${null}%#s"
<console>:8: error: Requires an instance of java.util.Formattable
              f"${null}%#s"
                         ^

scala> f"${42000}%,,d"
<console>:8: error: Duplicate flag ','
              f"${42000}%,,d"
                          ^

scala> f"${3.14}%f %<d"
<console>:8: error: Specifier '%<d' not compatible with indexed arg
              f"${3.14}%f %<d"
              ^

FormatInterpolator checks that indexed specifiers (like %3$d) refer to arguments that are compatible with the specifier. When arguments are literal constants, aka constant value expressions, they are inlined using the specifier. Other checks are added or improved, including a check for duplicate flags. Sample debug output showing that only one arg is evaluated at runtime: ``` scala> :load -v test.script Loading test.script... scala> def x = System.currentTimeMillis ; final val i = 17 ; final val pi = 3.14 ; final val s = "hello, world" x: Long i: Int(17) = 17 pi: Double(3.14) = 3.14 s: String("hello, world") = hello, world scala> f"$x $pi%f %<s $i%d $s %1$$d" Format '%s 3.140000 3.14 17 hello, world %1$d' with 1 tmps and 1 args res0: String = 1422731811259 3.140000 3.14 17 hello, world 1422731811259 ``` It doesn't attempt (bother) to inline enum symbols and class constants. The "incompatible arg" error caret position could be improved.

For inlining purposes, use ConstantFolder and constant.convertTo to handle expressions and conversions.

The string conversion only requires a top type, but preserve the actual type so that indexed references work. As an edge case, args typed `Any` are downcast to `AnyRef` for the call to `String.format`.

One might hope that \a\v\e means something besides aloha. This commit adds a suggestion for the ASCII control codes.

adriaanm · 2015-02-19T00:17:25Z

Since the 2.11.6 deadline is extremely nigh, I've tentatively postponed this PR (along with other recent ones) to 2.11.7. I'll make a pass through reviewed 2.11.7 PRs on Friday and move them back to 2.11.6 where feasible.

In the mean time, feel free to let me know if you'd like some help getting this reviewed by then.

soc · 2015-02-22T01:11:45Z

@som-snytt Would it make sense to macroify the s-interpolator, too? (Btw, any plans for regextractor?)

retronym · 2015-03-24T05:01:25Z

Hmmmm.

quick.comp:
    [mkdir] Created dir: /home/jenkins/workspace/scala-2.11.x-validate-test@2/build/quick/classes/compiler
[quick.compiler] Compiling 316 files to /home/jenkins/workspace/scala-2.11.x-validate-test@2/build/quick/classes/compiler
[quick.compiler] /home/jenkins/workspace/scala-2.11.x-validate-test@2/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala:868: warning: match may not be exhaustive.
[quick.compiler] It would fail on the following input: List(_)
[quick.compiler]         seenTypes(i) match {
[quick.compiler]                  ^
[quick.compiler] error: symbol value x2 does not exist in scala.tools.nsc.typechecker.Implicits$ImplicitSearch.isImpossibleSubType
[quick.compiler] error: scala.reflect.internal.FatalError: symbol value x2 does not exist in scala.tools.nsc.typechecker.Implicits$ImplicitSearch.isImpossibleSubType
[quick.compiler]    at scala.reflect.internal.Reporting$class.abort(Reporting.scala:59)
[quick.compiler]    at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:17)
[quick.compiler]    at scala.tools.nsc.backend.icode.GenICode$ICodePhase.genLoadIdent$1(GenICode.scala:885)

/rebuild

This error has been haunting us recently, firstly on Greg's machine when compiling the compiler using the new SBT build, and more recently during PR validation in scala#4316. This commit will output an error like: symbol value c#16812 does not exist in Macro.impl, which contains locals value a#16810, value b#16811 I've included symbol IDs in the hope that they will prove useful. It seems that synthetic identifiers generated by the pattern matcher are often seen in the vicinity of this bug.

retronym · 2015-03-24T05:25:41Z

I've added a better diagnostic for that compiler crash in #4397. I can't see how it is related to this change.

/rebuild

som-snytt · 2015-03-24T05:26:02Z

@retronym I did a quick rebase earlier today, something might have got munged...

som-snytt · 2015-03-24T06:31:38Z

FYI, I reverted this to the unrebased, and I'm trying out a fresh rebase on the other PR.

It was nice getting the green checks here for free. Jenkins has elephantine memory. Pachydermal recall.

retronym · 2015-03-24T06:37:28Z

/rebuild for the less green ones.

som-snytt added 2 commits February 10, 2015 10:03

SI-8650 F-interpolator folds constants

df7037d

For inlining purposes, use ConstantFolder and constant.convertTo to handle expressions and conversions.

scala-jenkins added this to the 2.11.6 milestone Feb 11, 2015

som-snytt mentioned this pull request Feb 11, 2015

SI-8650 F-interpolator inlines literals #4282

Closed

som-snytt added 3 commits February 10, 2015 21:48

SI-8650 F-interpolator doesn't lose type on %s

6f3951c

The string conversion only requires a top type, but preserve the actual type so that indexed references work. As an edge case, args typed `Any` are downcast to `AnyRef` for the call to `String.format`.

SI-8650 F-interpolator does syms and tpes

62129e8

SI-6476 Improve message for unsupported escapes

87a0adb

One might hope that \a\v\e means something besides aloha. This commit adds a suggestion for the ASCII control codes.

som-snytt mentioned this pull request Feb 11, 2015

SI-6476 Accept escaped quotes in interp strings #4308

Closed

adriaanm modified the milestones: 2.11.6, 2.11.7 Feb 19, 2015

som-snytt force-pushed the issue/8650-noeval branch from 87a0adb to f38f483 Compare February 25, 2015 22:30

som-snytt force-pushed the issue/8650-noeval branch from f38f483 to fbd01b6 Compare March 23, 2015 21:00

retronym mentioned this pull request Mar 24, 2015

Improve diagnostic error on failed genLoadIdent #4397

Merged

som-snytt force-pushed the issue/8650-noeval branch from fbd01b6 to 87a0adb Compare March 24, 2015 05:33

som-snytt mentioned this pull request Mar 24, 2015

SI-8650 F-interpolator inlines literals #4398

Closed

retronym closed this Mar 24, 2015

scabug mentioned this pull request Apr 7, 2017

String interpolation with constants scala/bug#8650

Closed

som-snytt deleted the issue/8650-noeval branch December 19, 2020 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SI-8650 F-interpolator inlines literals#4316

SI-8650 F-interpolator inlines literals#4316
som-snytt wants to merge 5 commits intoscala:2.11.xfrom
som-snytt:issue/8650-noeval

som-snytt commented Feb 11, 2015

Uh oh!

adriaanm commented Feb 19, 2015

Uh oh!

soc commented Feb 22, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

som-snytt commented Mar 24, 2015

Uh oh!

som-snytt commented Mar 24, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

som-snytt commented Feb 11, 2015

Uh oh!

adriaanm commented Feb 19, 2015

Uh oh!

soc commented Feb 22, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

som-snytt commented Mar 24, 2015

Uh oh!

som-snytt commented Mar 24, 2015

Uh oh!

retronym commented Mar 24, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants