diff options
author | Lukas Rytz <lukas.rytz@gmail.com> | 2014-10-14 16:35:35 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@gmail.com> | 2014-10-15 08:15:01 +0200 |
commit | e96d1a4d346b1ebac46796307a0887d7b7c4fccf (patch) | |
tree | 591ac2ba479c358b2913d1bf5d5f912934cb1833 | |
parent | 2b5df373638d08204b71258928289f6b39e25d5f (diff) | |
download | scala-e96d1a4d346b1ebac46796307a0887d7b7c4fccf.tar.gz scala-e96d1a4d346b1ebac46796307a0887d7b7c4fccf.tar.bz2 scala-e96d1a4d346b1ebac46796307a0887d7b7c4fccf.zip |
SI-8907 Don't force symbol info in isModuleNotMethod
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`.
0ccdb151f 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.
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Flatten.scala | 7 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Symbols.scala | 44 | ||||
-rw-r--r-- | test/files/run/t8907.scala | 39 |
3 files changed, 66 insertions, 24 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index fa53ef48b5..4662ef6224 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -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 diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index b0c23ef45d..2c039ab5a7 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -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 diff --git a/test/files/run/t8907.scala b/test/files/run/t8907.scala new file mode 100644 index 0000000000..7952ac82d9 --- /dev/null +++ b/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()) + } +} |