Skip to content

Commit

Permalink
SI-8907 Don't force symbol info in isModuleNotMethod
Browse files Browse the repository at this point in the history
Test case by Jason.

RefChecks adds the lateMETHOD flag lazily in its info transformer.
This means that forcing the `sym.info` may change the value of
`sym.isMethod`.

0ccdb15 introduced a check to force the info in isModuleNotMethod,
but it turns out this leads to errors on stub symbols (SI-8907).

The responsibility to force info is transferred to callers, which
is the case for other operations on symbols, too.
  • Loading branch information
lrytz committed Oct 15, 2014
1 parent 2b5df37 commit e96d1a4
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 24 deletions.
7 changes: 5 additions & 2 deletions src/compiler/scala/tools/nsc/transform/Flatten.scala
Expand Up @@ -77,8 +77,11 @@ abstract class Flatten extends InfoTransform {
if (sym.isTerm && !sym.isStaticModule) {
decls1 enter sym
if (sym.isModule) {
// Nested, non-static moduls are transformed to methods.
assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks")
// In theory, we could assert(sym.isMethod), because nested, non-static moduls are
// transformed to methods (lateMETHOD flag added in RefChecks). But this requires
// forcing sym.info (see comment on isModuleNotMethod), which forces stub symbols
// too eagerly (SI-8907).

// Note that module classes are not entered into the 'decls' of the ClassInfoType
// of the outer class, only the module symbols are. So the current loop does
// not visit module classes. Therefore we set the LIFTED flag here for module
Expand Down
44 changes: 22 additions & 22 deletions src/reflect/scala/reflect/internal/Symbols.scala
Expand Up @@ -735,31 +735,31 @@ trait Symbols extends api.Symbols { self: SymbolTable =>

final def hasGetter = isTerm && nme.isLocalName(name)

/** A little explanation for this confusing situation.
* Nested modules which have no static owner when ModuleDefs
* are eliminated (refchecks) are given the lateMETHOD flag,
* which makes them appear as methods after refchecks.
* Here's an example where one can see all four of FF FT TF TT
* for (isStatic, isMethod) at various phases.
/**
* Nested modules which have no static owner when ModuleDefs are eliminated (refchecks) are
* given the lateMETHOD flag, which makes them appear as methods after refchecks.
*
* Note: the lateMETHOD flag is added lazily in the info transformer of the RefChecks phase.
* This means that forcing the `sym.info` may change the value of `sym.isMethod`. Forcing the
* info is in the responsability of the caller. Doing it eagerly here was tried (0ccdb151f) but
* has proven to lead to bugs (SI-8907).
*
* trait A1 { case class Quux() }
* object A2 extends A1 { object Flax }
* // -- namer object Quux in trait A1
* // -M flatten object Quux in trait A1
* // S- flatten object Flax in object A2
* // -M posterasure object Quux in trait A1
* // -M jvm object Quux in trait A1
* // SM jvm object Quux in object A2
* Here's an example where one can see all four of FF FT TF TT for (isStatic, isMethod) at
* various phases.
*
* So "isModuleNotMethod" exists not for its achievement in
* brevity, but to encapsulate the relevant condition.
* trait A1 { case class Quux() }
* object A2 extends A1 { object Flax }
* // -- namer object Quux in trait A1
* // -M flatten object Quux in trait A1
* // S- flatten object Flax in object A2
* // -M posterasure object Quux in trait A1
* // -M jvm object Quux in trait A1
* // SM jvm object Quux in object A2
*
* So "isModuleNotMethod" exists not for its achievement in brevity, but to encapsulate the
* relevant condition.
*/
def isModuleNotMethod = {
if (isModule) {
if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there.
!isMethod
} else false
}
def isModuleNotMethod = isModule && !isMethod

// After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should
// be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module
Expand Down
39 changes: 39 additions & 0 deletions test/files/run/t8907.scala
@@ -0,0 +1,39 @@
import scala.tools.partest._
import java.io.File

object Test extends StoreReporterDirectTest {
def code = ???

def compileCode(code: String) = {
val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code)
}

def show(): Unit = {
compileCode("""
class C { class Inner }
class D {
object O {
def foo(c: C)(i: c.Inner): c.Inner = ???
}
}
""")
assert(filteredInfos.isEmpty, filteredInfos)
deleteClass("C")
compileCode("""
class E {
def foo = {
(null: D).toString
}
}
""")
assert(storeReporter.infos.isEmpty, storeReporter.infos.mkString("\n")) // Included a MissingRequirementError before.
}

def deleteClass(name: String) {
val classFile = new File(testOutput.path, name + ".class")
assert(classFile.exists)
assert(classFile.delete())
}
}

0 comments on commit e96d1a4

Please sign in to comment.