From 426011b83626ff15233593448377778800735294 Mon Sep 17 00:00:00 2001 From: lloydmeta Date: Sun, 8 Jul 2018 13:23:27 +0900 Subject: [PATCH 1/2] Fix #4732: Root the desugared call to StringContext * Make desugared `s""` call `_root_.scala.StringContext` instead of just `StringContext` to fix the initial issue * Add a unit test to lock in the change * Update StringInterpolatorOpt#unapply so that the StringInterpolator optimistation can work with the new change --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 9 +++++++-- .../transform/localopt/StringInterpolatorOpt.scala | 2 +- tests/run/rooted_stringcontext.check | 1 + tests/run/rooted_stringcontext.scala | 10 ++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 tests/run/rooted_stringcontext.check create mode 100644 tests/run/rooted_stringcontext.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 5f42c748bd33..760b68fd3ad4 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1109,8 +1109,13 @@ object desugar { case Block(Nil, expr) => expr // important for interpolated string as patterns, see i1773.scala case t => t } - - Apply(Select(Apply(Ident(nme.StringContext), strs), id), elems) + // This is a deliberate departure from scalac, where StringContext is not rooted (See #4732) + val rootedStringContext = { + val rootPkg = Ident(nme.ROOTPKG) + val scalaPkg = Select(rootPkg, nme.scala_) + Select(scalaPkg, nme.StringContext) + } + Apply(Select(Apply(rootedStringContext, strs), id), elems) case InfixOp(l, op, r) => if (ctx.mode is Mode.Type) AppliedTypeTree(op, l :: r :: Nil) // op[l, r] diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index 293096a2b6e7..d4e42996d614 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -36,7 +36,7 @@ class StringInterpolatorOpt extends MiniPhase { def unapply(tree: Select)(implicit ctx: Context): Boolean = { tree.symbol.eq(defn.StringContextModule_apply) && { val qualifier = tree.qualifier - qualifier.isInstanceOf[Ident] && qualifier.symbol.eq(defn.StringContextModule) + qualifier.isInstanceOf[Select] && qualifier.symbol.eq(defn.StringContextModule) } } } diff --git a/tests/run/rooted_stringcontext.check b/tests/run/rooted_stringcontext.check new file mode 100644 index 000000000000..930d34fb3f1d --- /dev/null +++ b/tests/run/rooted_stringcontext.check @@ -0,0 +1 @@ +Answer: 42 \ No newline at end of file diff --git a/tests/run/rooted_stringcontext.scala b/tests/run/rooted_stringcontext.scala new file mode 100644 index 000000000000..39799e68ba7b --- /dev/null +++ b/tests/run/rooted_stringcontext.scala @@ -0,0 +1,10 @@ +object StringContext { + val i = 42 + val s = s"Answer: $i" +} + +object Test { + def main(args: Array[String]): Unit = { + println(StringContext.s) + } +} \ No newline at end of file From 086c8913a7a4ea093e76901e059fc4d3b05fc6d9 Mon Sep 17 00:00:00 2001 From: lloydmeta Date: Tue, 10 Jul 2018 19:37:31 +0900 Subject: [PATCH 2/2] Address review comments * Simplify grabbing of rooted handle to StringContext * Simplify StringContext tree unapply * Remove check file for test and use an assert instead --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 7 +------ .../dotc/transform/localopt/StringInterpolatorOpt.scala | 6 ++---- tests/run/rooted_stringcontext.check | 1 - tests/run/rooted_stringcontext.scala | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) delete mode 100644 tests/run/rooted_stringcontext.check diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 760b68fd3ad4..6b7343913339 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -1110,12 +1110,7 @@ object desugar { case t => t } // This is a deliberate departure from scalac, where StringContext is not rooted (See #4732) - val rootedStringContext = { - val rootPkg = Ident(nme.ROOTPKG) - val scalaPkg = Select(rootPkg, nme.scala_) - Select(scalaPkg, nme.StringContext) - } - Apply(Select(Apply(rootedStringContext, strs), id), elems) + Apply(Select(Apply(scalaDot(nme.StringContext), strs), id), elems) case InfixOp(l, op, r) => if (ctx.mode is Mode.Type) AppliedTypeTree(op, l :: r :: Nil) // op[l, r] diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index d4e42996d614..f5bd2d64b319 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -34,10 +34,8 @@ class StringInterpolatorOpt extends MiniPhase { private object StringContextApply { def unapply(tree: Select)(implicit ctx: Context): Boolean = { - tree.symbol.eq(defn.StringContextModule_apply) && { - val qualifier = tree.qualifier - qualifier.isInstanceOf[Select] && qualifier.symbol.eq(defn.StringContextModule) - } + tree.symbol.eq(defn.StringContextModule_apply) && + tree.qualifier.symbol.eq(defn.StringContextModule) } } diff --git a/tests/run/rooted_stringcontext.check b/tests/run/rooted_stringcontext.check deleted file mode 100644 index 930d34fb3f1d..000000000000 --- a/tests/run/rooted_stringcontext.check +++ /dev/null @@ -1 +0,0 @@ -Answer: 42 \ No newline at end of file diff --git a/tests/run/rooted_stringcontext.scala b/tests/run/rooted_stringcontext.scala index 39799e68ba7b..65f7a7e064fd 100644 --- a/tests/run/rooted_stringcontext.scala +++ b/tests/run/rooted_stringcontext.scala @@ -5,6 +5,6 @@ object StringContext { object Test { def main(args: Array[String]): Unit = { - println(StringContext.s) + assert(StringContext.s == "Answer: 42") } } \ No newline at end of file