Browse files

SI-8111 Repair symbol owners after abandoned named-/default-args

Names/Defaults eagerly transforms an application with temporaries
to maintain evaluation order, and dutifully changes owners of
symbols along the way.

However, if this approach doesn't work out, we throw away this
and try a auto-tupling. However, we an still witness symbols
owned by the temporaries.

This commit records which symbols are owned by the context.owner
before `transformNamedApplication`, and rolls back the changes
before `tryTupleApply`.

Perhaps a better approach would be to separate the names/defaults
applicability checks from the evaluation-order-preserving transform,
and only call the latter after we have decided to go that way.
  • Loading branch information...
1 parent 370d6d6 commit 2c770ae31a71d5beece4753ce4d43265036ee477 @retronym retronym committed Jan 5, 2014
Showing with 42 additions and 0 deletions.
  1. +18 −0 src/compiler/scala/tools/nsc/typechecker/Typers.scala
  2. +24 −0 test/files/pos/t8111.scala
View
18 src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -3269,6 +3269,23 @@ trait Typers extends Modes with Adaptations with Tags {
// calls to the default getters. Example:
// foo[Int](a)() ==> foo[Int](a)(b = foo$qual.foo$default$2[Int](a))
checkNotMacro()
+
+ // SI-8111 transformNamedApplication eagerly shuffles around the application to preserve
+ // evaluation order. During this process, it calls `changeOwner` on symbols that
+ // are transplanted underneath synthetic temporary vals.
+ //
+ // Here, we keep track of the symbols owned by `context.owner` to enable us to
+ // rollback. Note that duplicating trees would not be enough to fix this problem,
+ // we would also need to clone local symbols in the duplicated tree to truly isolate
+ // things.
+ def ownerOf(sym: Symbol) = if (sym == null || sym == NoSymbol) NoSymbol else sym.owner
+ val symsOwnedByContextOwner = tree.collect {
+ case t @ (_: DefTree | _: Function) if ownerOf(t.symbol) == context.owner => t.symbol
+ }
+ def rollbackNamesDefaultsOwnerChanges() {
+ symsOwnedByContextOwner foreach (_.owner = context.owner)
+ }
+
val fun1 = transformNamedApplication(Typer.this, mode, pt)(fun, x => x)
if (fun1.isErroneous) duplErrTree
else {
@@ -3297,6 +3314,7 @@ trait Typers extends Modes with Adaptations with Tags {
if (!(context.diagnostic contains note)) context.diagnostic = note :: context.diagnostic
doTypedApply(tree, if (blockIsEmpty) fun else fun1, allArgs, mode, pt)
} else {
+ rollbackNamesDefaultsOwnerChanges()
tryTupleApply getOrElse duplErrorTree(NotEnoughArgsError(tree, fun, missing))
}
}
View
24 test/files/pos/t8111.scala
@@ -0,0 +1,24 @@
+trait T {
+
+ def crashy(ma: Any) {
+ // okay
+ val f1 = (u: Unit) => ma
+ foo(f1)()
+ foo((u: Unit) => ma)
+ foo(0, (u: Any) => ma) apply ()
+
+ // crash due to side effects on the onwer of the symbol in the
+ // qualifier or arguments of the application during an abandoned
+ // names/defaults transform. The code type checkes because of
+ // autp-tupling which promotes and empty parmater list to `(): Unit`
+ foo((u: Any) => ma)()
+
+ {{(u: Any) => ma}; this}.foo(0)()
+
+ foo({def foo = ma; 0})()
+
+ {def foo = ma; this}.foo(0)()
+ }
+
+ def foo(f: Any): Any => Any
+}

0 comments on commit 2c770ae

Please sign in to comment.