Skip to content
This repository
Browse code

SI-4859 Don't elide qualifiers when selecting nested modules.

Otherwise we fail to throw in:

   {???; Predef}.DummyImplicit.dummyImplicit

We still elide the initialization of `Outer` in `Outer.Inner.foo`
as before, although that seems a little dubious to me.

In total, we had to change RefChecks, Flatten, and GenICode
to effect this change. A recently fixed bug in tail call elimination
was also due to assuming that the the qualifier of a Select node
wasn't worthy of traversal.  Let's keep a close eye out for more
instances of this problem.
  • Loading branch information...
commit f21b1ce7fda9022d6d805a708882c5a2ab241f41 1 parent eb4b065
Jason Zaugg retronym authored
11 src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -835,13 +835,18 @@ abstract class GenICode extends SubComponent {
835 835 generatedType = toTypeKind(sym.info)
836 836 val hostClass = findHostClass(qualifier.tpe, sym)
837 837 log(s"Host class of $sym with qual $qualifier (${qualifier.tpe}) is $hostClass")
  838 + val qualSafeToInline = treeInfo isExprSafeToInline qualifier
  839 +
  840 + def genLoadQualUnlessInlinable: Context =
  841 + if (qualSafeToInline) ctx else genLoadQualifier(tree, ctx)
838 842
839 843 if (sym.isModule) {
840   - genLoadModule(ctx, tree)
  844 + genLoadModule(genLoadQualUnlessInlinable, tree)
841 845 }
842 846 else if (sym.isStaticMember) {
843   - ctx.bb.emit(LOAD_FIELD(sym, true) setHostClass hostClass, tree.pos)
844   - ctx
  847 + val ctx1 = genLoadQualUnlessInlinable
  848 + ctx1.bb.emit(LOAD_FIELD(sym, true) setHostClass hostClass, tree.pos)
  849 + ctx1
845 850 } else {
846 851 val ctx1 = genLoadQualifier(tree, ctx)
847 852 ctx1.bb.emit(LOAD_FIELD(sym, false) setHostClass hostClass, tree.pos)
9 src/compiler/scala/tools/nsc/transform/Flatten.scala
@@ -12,6 +12,7 @@ import scala.collection.mutable.ListBuffer
12 12
13 13 abstract class Flatten extends InfoTransform {
14 14 import global._
  15 + import treeInfo.isExprSafeToInline
15 16
16 17 /** the following two members override abstract members in Transform */
17 18 val phaseName: String = "flatten"
@@ -117,7 +118,13 @@ abstract class Flatten extends InfoTransform {
117 118 liftedDefs(sym.enclosingTopLevelClass.owner) += tree
118 119 EmptyTree
119 120 case Select(qual, name) if sym.isStaticModule && !sym.isTopLevel =>
120   - exitingFlatten(atPos(tree.pos)(gen.mkAttributedRef(sym)))
  121 + exitingFlatten {
  122 + atPos(tree.pos) {
  123 + val ref = gen.mkAttributedRef(sym)
  124 + if (isExprSafeToInline(qual)) ref
  125 + else Block(List(qual), ref).setType(tree.tpe) // need to execute the qualifier but refer directly to the lifted module.
  126 + }
  127 + }
121 128 case _ =>
122 129 tree
123 130 }
3  src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -1389,7 +1389,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
1389 1389 case TypeApply(fun, targs) =>
1390 1390 isClassTypeAccessible(fun)
1391 1391 case Select(module, apply) =>
1392   - ( // SI-4859 `CaseClass1().InnerCaseClass2()` must not be rewritten to `new InnerCaseClass2()`
  1392 + ( // SI-4859 `CaseClass1().InnerCaseClass2()` must not be rewritten to `new InnerCaseClass2()`;
  1393 + // {expr; Outer}.Inner() must not be rewritten to `new Outer.Inner()`.
1393 1394 treeInfo.isExprSafeToInline(module) &&
1394 1395 // SI-5626 Classes in refinement types cannot be constructed with `new`. In this case,
1395 1396 // the companion class is actually not a ClassSymbol, but a reference to an abstract type.
8 test/files/run/t4859.check
... ... @@ -0,0 +1,8 @@
  1 +Inner
  2 +Inner.i
  3 +About to reference Inner.i
  4 +Outer
  5 +Inner.i
  6 +About to reference O.N
  7 +About to reference O.N
  8 +About to reference O.N.apply()
29 test/files/run/t4859.scala
... ... @@ -0,0 +1,29 @@
  1 +object O {
  2 + case class N()
  3 + object P
  4 +}
  5 +
  6 +object Outer {
  7 + println("Outer")
  8 + object Inner {
  9 + println("Inner")
  10 + def i {
  11 + println("Inner.i")
  12 + }
  13 + }
  14 +}
  15 +
  16 +object Test {
  17 + def main(args: Array[String]) {
  18 + Outer.Inner.i // we still don't initiialize Outer here (but should we?)
  19 +
  20 + {println("About to reference Inner.i"); Outer}.Inner.i // Outer will be initialized.
  21 +
  22 + {println("About to reference O.N" ); O}.N
  23 +
  24 + {println("About to reference O.N" ); O}.N
  25 +
  26 + {println("About to reference O.N.apply()"); O}.N.apply()
  27 + }
  28 +}
  29 +

0 comments on commit f21b1ce

Please sign in to comment.
Something went wrong with that request. Please try again.