Skip to content

Fix #2809: Invalidate and unlink symbols for artifact classfiles #2830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,17 @@ object SymDenotations {
final def markAbsent(): Unit =
myInfo = NoType

/** Is symbol known to not exist, or potentially not completed yet? */
final def unforcedIsAbsent(implicit ctx: Context): Boolean =
myInfo == NoType ||
(this is (ModuleVal, butNot = Package)) && moduleClass.unforcedIsAbsent

/** Is symbol known to not exist? */
final def isAbsent(implicit ctx: Context): Boolean =
final def isAbsent(implicit ctx: Context): Boolean = {
ensureCompleted()
myInfo == NoType ||
(this is (ModuleVal, butNot = Package)) && moduleClass.isAbsent
}

/** Is this symbol the root class or its companion object? */
final def isRoot: Boolean =
Expand Down Expand Up @@ -563,7 +570,7 @@ object SymDenotations {
final def isCoDefinedWith(that: Symbol)(implicit ctx: Context) =
(this.effectiveOwner == that.effectiveOwner) &&
( !(this.effectiveOwner is PackageClass)
|| this.isAbsent || that.isAbsent
|| this.unforcedIsAbsent || that.unforcedIsAbsent
|| { // check if they are defined in the same file(or a jar)
val thisFile = this.symbol.associatedFile
val thatFile = that.symbol.associatedFile
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ trait Symbols { this: Context =>
val companionMethodFlags = Flags.Synthetic | Flags.Private | Flags.Method

def synthesizeCompanionMethod(name: Name, target: SymDenotation, owner: SymDenotation)(implicit ctx: Context) =
if (owner.exists && target.exists && !owner.isAbsent && !target.isAbsent) {
if (owner.exists && target.exists && !owner.unforcedIsAbsent && !target.unforcedIsAbsent) {
val existing = owner.unforcedDecls.lookup(name)

existing.orElse{
Expand Down
29 changes: 29 additions & 0 deletions compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import scala.util.control.NonFatal
object ClassfileParser {
/** Marker trait for unpicklers that can be embedded in classfiles. */
trait Embedded

/** Indicate that there is nothing to unpickle and the corresponding symbols can
* be invalidated. */
object NoEmbedded extends Embedded
}

class ClassfileParser(
Expand Down Expand Up @@ -147,6 +151,15 @@ class ClassfileParser(

setClassInfo(classRoot, classInfo)
setClassInfo(moduleRoot, staticInfo)
} else if (result == Some(NoEmbedded)) {
for (sym <- List(moduleRoot.sourceModule.symbol, moduleRoot.symbol, classRoot.symbol)) {
classRoot.owner.asClass.delete(sym)
if (classRoot.owner == defn.ScalaShadowingPackageClass) {
// Symbols in scalaShadowing are also added to scala
defn.ScalaPackageClass.delete(sym)
}
sym.markAbsent()
}
}

// eager load java enum definitions for exhaustivity check of pattern match
Expand Down Expand Up @@ -700,6 +713,10 @@ class ClassfileParser(
}
}

// Nothing$ and Null$ were incorrectly emitted with a Scala attribute
// instead of ScalaSignature before 2.13.0-M2, see https://github.com/scala/scala/pull/5952
private[this] val scalaUnpickleWhitelist = List(tpnme.nothingClass, tpnme.nullClass)

/** Parse inner classes. Expects `in.bp` to point to the superclass entry.
* Restores the old `bp`.
* @return true iff classfile is from Scala, so no Java info needs to be read.
Expand Down Expand Up @@ -760,6 +777,18 @@ class ClassfileParser(
return unpickleTASTY(in.nextBytes(attrLen))
}

if (scan(tpnme.ScalaATTR) && !scalaUnpickleWhitelist.contains(classRoot.name)) {
// To understand the situation, it's helpful to know that:
// - Scalac emits the `ScalaSig` attribute for classfiles with pickled information
// and the `Scala` attribute for everything else.
// - Dotty emits the `TASTY` attribute for classfiles with pickled information
// and the `Scala` attribute for _every_ classfile.
//
// Therefore, if the `Scala` attribute is present but the `TASTY`
// attribute isn't, this classfile is a compilation artifact.
return Some(NoEmbedded)
}

if (scan(tpnme.RuntimeAnnotationATTR)) {
val attrLen = in.nextInt
val nAnnots = in.nextChar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ object Interactive {
val boundaryCtx = ctx.withOwner(boundary)
try
prefix.memberDenots(completionsFilter, (name, buf) =>
buf ++= prefix.member(name).altsWith(_.symbol.isAccessibleFrom(prefix)(boundaryCtx))
buf ++= prefix.member(name).altsWith(d => !d.isAbsent && d.symbol.isAccessibleFrom(prefix)(boundaryCtx))
).map(_.symbol).toList
catch {
case ex: TypeError => Nil
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Pickler extends Phase {
/** Drop any elements of this list that are linked module classes of other elements in the list */
private def dropCompanionModuleClasses(clss: List[ClassSymbol])(implicit ctx: Context): List[ClassSymbol] = {
val companionModuleClasses =
clss.filterNot(_ is Module).map(_.linkedClass).filterNot(_.isAbsent)
clss.filterNot(_ is Module).map(_.linkedClass).filterNot(_.unforcedIsAbsent)
clss.filterNot(companionModuleClasses.contains)
}

Expand Down
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,7 @@ trait TypeAssigner {
final def reallyExists(denot: Denotation)(implicit ctx: Context): Boolean = try
denot match {
case denot: SymDenotation =>
denot.exists && {
denot.ensureCompleted()
!denot.isAbsent
}
denot.exists && !denot.isAbsent
case denot: SingleDenotation =>
val sym = denot.symbol
(sym eq NoSymbol) || reallyExists(sym.denot)
Expand Down
15 changes: 15 additions & 0 deletions tests/neg/classfile-artifacts.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// For some reason, if the imports are not scoped, only the first import error
// is reported

class A {
import scala.languageFeature$experimental$._ // error
}

class B {
import scala.language$Scala2$._ // error
}

class C {
import scala.languageFeature$._ // error
}