Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

SI-8010 Fix regression in erasure double definition checks

Calls to `Symbol#info` during scope iteration considered harmful.

Looks like calling `info` during this Scope iteration is triggering
the ExplicitOuter info transformer, which "makes all super accessors
and modules in traits non-private, mangling their names.". This name
change necessitates a rehashing of the owning scope, which I suspect
is enough to corrupt the ScopeEntry-s being traversed in
`checkNoDeclaredDoubleDefs`.

The upshot was that we encountered the same symbol twice, which was
reported as being a double-definition.

This problem only showed up after 086702d, which did nothing
worse then change the order in which `{e, e1}.sym.info` were
forced.

I inspected SymbolPairs/OverridingPairs which *appear* to be immune
as they only test flags during scope iteration; infos are not used
until later, at which point we're iterating a temporary scope that
isn't part of the type of the owner of the symbols.
  • Loading branch information...
commit 9036f774bc834acd5b71484d2fa3882896ffd3ce 1 parent 7c1d114
@retronym retronym authored
View
13 src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -918,11 +918,24 @@ abstract class Erasure extends AddInterfaces
}
val decls = root.info.decls
+
+ // SI-8010 force infos, otherwise makeNotPrivate in ExplicitOuter info transformer can trigger
+ // a scope rehash while were iterating and we can see the same entry twice!
+ // Inspection of SymbolPairs (the basis of OverridingPairs), suggests that it is immune
+ // from this sort of bug as it copies the symbols into a temporary scope *before* any calls to `.info`,
+ // ie, no variant of it calls `info` or `tpe` in `SymbolPair#exclude`.
+ //
+ // Why not just create a temporary scope here? We need to force the name changes in any case before
+ // we do these checks, so that we're comparing same-named methods based on the expanded names that actually
+ // end up in the bytecode.
+ afterPostErasure(decls.foreach(_.info))
+
var e = decls.elems
while (e ne null) {
if (e.sym.isTerm) {
var e1 = decls.lookupNextEntry(e)
while (e1 ne null) {
+ assert(e.sym ne e1.sym, s"Internal error: encountered ${e.sym.debugLocationString} twice during scope traversal. This might be related to SI-8010.")
if (sameTypeAfterErasure(e1.sym, e.sym)) doubleDefError(e.sym, e1.sym)
e1 = decls.lookupNextEntry(e1)
}
View
22 test/files/run/t8010.scala
@@ -0,0 +1,22 @@
+trait Base {
+ def t = 1
+ def t(n: Int) = n
+ def bt = 2
+ def bt(n: Int) = n
+}
+trait Derived extends Base {
+ // was: double defintion error
+ override def t = 1 + super.t
+ override def t(n: Int) = 1 + super.t(n)
+ override def bt = 1 + super.bt
+ override def bt(n: Int) = 1 + super.bt(n)
+}
+
+object Test extends App {
+ val d = new Derived {}
+ // not the focus of thie bug, but let's just check the runtime behaviour while we're here.
+ assert(d.t == 2)
+ assert(d.t(1) == 2)
+ assert(d.bt == 3)
+ assert(d.bt(1) == 2)
+}
Please sign in to comment.
Something went wrong with that request. Please try again.