Skip to content
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

Fix #9970: Reconcile how Scala 3 and 2 decide whether a trait has $init$. #10530

Merged
merged 1 commit into from
Dec 9, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped]
/** The largest subset of {NoInits, PureInterface} that a
* trait or class enclosing this statement can have as flags.
*/
def defKind(tree: Tree)(using Context): FlagSet = unsplice(tree) match {
private def defKind(tree: Tree)(using Context): FlagSet = unsplice(tree) match {
case EmptyTree | _: Import => NoInitsInterface
case tree: TypeDef => if (tree.isClassDef) NoInits else NoInitsInterface
case tree: DefDef =>
Expand Down Expand Up @@ -402,8 +402,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
|| sym.owner == defn.StringClass
|| defn.pureMethods.contains(sym)
if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol) // A constant expression with pure arguments is pure.
|| (fn.symbol.isStableMember && !fn.symbol.is(Lazy))
|| fn.symbol.isPrimaryConstructor && fn.symbol.owner.isNoInitsClass) // TODO: include in isStable?
|| (fn.symbol.isStableMember && !fn.symbol.is(Lazy))) // constructors of no-inits classes are stable
minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
else if (fn.symbol.is(Erased)) Pure
else if (fn.symbol.isStableMember) /* && fn.symbol.is(Lazy) */
Expand Down Expand Up @@ -459,7 +458,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
else if tree.tpe.isInstanceOf[ConstantType] then PurePath
else if (!sym.isStableMember) Impure
else if (sym.is(Module))
if (sym.moduleClass.isNoInitsClass) PurePath else IdempotentPath
if (sym.moduleClass.isNoInitsRealClass) PurePath else IdempotentPath
else if (sym.is(Lazy)) IdempotentPath
else if sym.isAllOf(Inline | Param) then Impure
else PurePath
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ class Definitions {
}

private def completeClass(cls: ClassSymbol, ensureCtor: Boolean = true): ClassSymbol = {
if (ensureCtor) ensureConstructor(cls, EmptyScope)
if (ensureCtor) ensureConstructor(cls, cls.denot.asClass, EmptyScope)
if (cls.linkedClass.exists) cls.linkedClass.markAbsent()
cls
}
Expand Down
14 changes: 11 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,14 @@ object Flags {
*/
val (Abstract @ _, _, _) = newFlags(23, "abstract")

/** Lazy val or method is known or assumed to be stable and realizable */
/** Lazy val or method is known or assumed to be stable and realizable.
*
* For a trait constructor, this is set if and only if owner.is(NoInits),
sjrd marked this conversation as resolved.
Show resolved Hide resolved
* including for Java interfaces and for Scala 2 traits. It will be used by
*
* - the purity analysis used by the inliner to decide whether it is safe to elide, and
* - the TASTy reader of Scala 2.13, to determine whether there is a $init$ method.
*/
val (_, StableRealizable @ _, _) = newFlags(24, "<stable>")

/** A case parameter accessor */
Expand All @@ -319,7 +326,9 @@ object Flags {

/** Variable is accessed from nested function
* /
* Trait does not have fields or initialization code.
* Trait does not have own fields or initialization code or class does not
* have own or inherited initialization code.
*
* Warning: NoInits is set during regular typer pass, should be tested only after typer.
*/
val (_, Captured @ _, NoInits @ _) = newFlags(32, "<captured>", "<noinits>")
Expand Down Expand Up @@ -536,7 +545,6 @@ object Flags {
val DeferredOrTermParamOrAccessor: FlagSet = Deferred | ParamAccessor | TermParam // term symbols without right-hand sides
val DeferredOrTypeParam: FlagSet = Deferred | TypeParam // type symbols without right-hand sides
val EnumValue: FlagSet = Enum | StableRealizable // A Scala enum value
val StableOrErased: FlagSet = Erased | StableRealizable // Assumed to be pure
val FinalOrInline: FlagSet = Final | Inline
val FinalOrModuleClass: FlagSet = Final | ModuleClass // A module class or a final class
val EffectivelyFinalFlags: FlagSet = Final | Private
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -725,11 +725,11 @@ object SymDenotations {
isType || is(StableRealizable) || exists && !isUnstableValue
}

/** Is this a denotation of a class that does not have - either direct or inherited -
/** Is this a denotation of a real class that does not have - either direct or inherited -
* initialization code?
*/
def isNoInitsClass(using Context): Boolean =
isClass &&
def isNoInitsRealClass(using Context): Boolean =
isRealClass &&
(asClass.baseClasses.forall(_.is(NoInits)) || defn.isAssuredNoInits(symbol))

/** Is this a "real" method? A real method is a method which is:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,14 @@ object Scala2Unpickler {
denot.info = PolyType.fromParams(denot.owner.typeParams, denot.info)
}

def ensureConstructor(cls: ClassSymbol, scope: Scope)(using Context): Unit = {
def ensureConstructor(cls: ClassSymbol, clsDenot: ClassDenotation, scope: Scope)(using Context): Unit = {
if (scope.lookup(nme.CONSTRUCTOR) == NoSymbol) {
val constr = newDefaultConstructor(cls)
// Scala 2 traits have a constructor iff they have initialization code
// In dotc we represent that as !StableRealizable, which is also owner.is(NoInits)
if clsDenot.flagsUNSAFE.is(Trait) then
constr.setFlag(StableRealizable)
clsDenot.setFlag(NoInits)
addConstructorTypeParams(constr)
cls.enter(constr, scope)
}
Expand Down Expand Up @@ -95,7 +100,7 @@ object Scala2Unpickler {
if (tsym.exists) tsym.setFlag(TypeParam)
else denot.enter(tparam, decls)
}
if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(denot.symbol.asClass, decls)
if (!denot.flagsUNSAFE.isAllOf(JavaModule)) ensureConstructor(cls, denot, decls)

val scalacCompanion = denot.classSymbol.scalacLinkedClass

Expand Down Expand Up @@ -465,11 +470,13 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
denot.setFlag(flags)
denot.resetFlag(Touched) // allow one more completion

// Temporary measure, as long as we do not read these classes from Tasty.
// Scala-2 classes don't have NoInits set even if they are pure. We override this
// for Product and Serializable so that case classes can be pure. A full solution
// requires that we read all Scala code from Tasty.
if (owner == defn.ScalaPackageClass && ((name eq tpnme.Serializable) || (name eq tpnme.Product)))
// Temporary measure, as long as we do not recompile these traits with Scala 3.
// Scala 2 is more aggressive when it comes to defining a $init$ method than Scala 3.
// Any concrete definition, even a `def`, causes a trait to receive a $init$ method
// to be created, even if it does not do anything, and hence causes not have the NoInits flag.
// We override this for Product so that cases classes can be pure.
// A full solution requires that we compile Product with Scala 3 in the future.
if (owner == defn.ScalaPackageClass && (name eq tpnme.Product))
denot.setFlag(NoInits)

denot.setPrivateWithin(privateWithin)
Expand Down
7 changes: 3 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -555,11 +555,10 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
meth.name == nme.apply &&
meth.flags.is(Synthetic) &&
meth.owner.linkedClass.is(Case) &&
cls.isNoInitsClass
cls.isNoInitsRealClass
}
if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol) // A constant expression with pure arguments is pure.
|| (fn.symbol.isStableMember && !fn.symbol.is(Lazy))
|| fn.symbol.isPrimaryConstructor && fn.symbol.owner.isNoInitsClass) // TODO: include in isStable?
|| (fn.symbol.isStableMember && !fn.symbol.is(Lazy))) // constructors of no-inits classes are stable
apply(fn) && args.forall(apply)
else if (isCaseClassApply)
args.forall(apply)
Expand Down Expand Up @@ -864,7 +863,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
def reduceProjection(tree: Tree)(using Context): Tree = {
if (ctx.debug) inlining.println(i"try reduce projection $tree")
tree match {
case Select(NewInstance(cls, args, prefix, precomputed), field) if cls.isNoInitsClass =>
case Select(NewInstance(cls, args, prefix, precomputed), field) if cls.isNoInitsRealClass =>
def matches(param: Symbol, selection: Symbol): Boolean =
param == selection || {
selection.name match {
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,10 @@ class Namer { typer: Typer =>
cls.info = avoidPrivateLeaks(cls)
cls.baseClasses.foreach(_.invalidateBaseTypeCache()) // we might have looked before and found nothing
cls.setNoInitsFlags(parentsKind(parents), untpd.bodyKind(rest))
if (cls.isNoInitsClass) cls.primaryConstructor.setFlag(StableRealizable)
val ctorStable =
if cls.is(Trait) then cls.is(NoInits)
else cls.isNoInitsRealClass
if ctorStable then cls.primaryConstructor.setFlag(StableRealizable)
processExports(using localCtx)
defn.patchStdLibClass(denot.asClass)
}
Expand Down
89 changes: 89 additions & 0 deletions tests/run-custom-args/tasty-inspector/i9970.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import scala.quoted._
import scala.tasty.inspector._

/* Test that the constructor of a trait has the StableRealizable trait if and
* only if the trait has NoInits. This is used by the TASTy reader in Scala 2
* to determine whether the constructor should be kept or discarded, and
* consequently whether calls to $init$ should be emitted for the trait or not.
*/

// Definitions to be inspected

trait I9970IOApp {
protected val foo = 23

def run(args: List[String]): Int

final def main(args: Array[String]): Unit = {
sys.exit(run(args.toList))
}
}

object I9970IOApp {
trait Simple extends I9970IOApp {
def run: Unit

final def run(args: List[String]): Int = {
run
0
}
}
}

// TASTy Inspector test boilerplate

object Test {
def main(args: Array[String]): Unit = {
// Artefact of the current test infrastructure
// TODO improve infrastructure to avoid needing this code on each test
val classpath = dotty.tools.dotc.util.ClasspathFromClassloader(this.getClass.getClassLoader).split(java.io.File.pathSeparator).find(_.contains("runWithCompiler")).get
val allTastyFiles = dotty.tools.io.Path(classpath).walkFilter(_.extension == "tasty").map(_.toString).toList
val tastyFiles = allTastyFiles.filter(_.contains("I9970"))

new TestInspector().inspectTastyFiles(tastyFiles)
}
}

// Inspector that performs the actual tests

class TestInspector() extends TastyInspector:

private var foundIOApp: Boolean = false
private var foundSimple: Boolean = false

protected def processCompilationUnit(using Quotes)(root: quotes.reflect.Tree): Unit =
foundIOApp = false
foundSimple = false
inspectClass(root)
// Sanity check to make sure that our pattern matches are not simply glossing over the things we want to test
assert(foundIOApp, "the inspector did not encounter IOApp")
assert(foundSimple, "the inspect did not encounter IOApp.Simple")

private def inspectClass(using Quotes)(tree: quotes.reflect.Tree): Unit =
import quotes.reflect._
tree match
case t: PackageClause =>
t.stats.foreach(inspectClass(_))

case t: ClassDef if t.name.endsWith("$") =>
t.body.foreach(inspectClass(_))

case t: ClassDef =>
t.name match
case "I9970IOApp" =>
foundIOApp = true
// Cannot test the following because NoInits is not part of the quotes API
//assert(!t.symbol.flags.is(Flags.NoInits))
assert(!t.constructor.symbol.flags.is(Flags.StableRealizable))

case "Simple" =>
foundSimple = true
// Cannot test the following because NoInits is not part of the quotes API
//assert(t.symbol.flags.is(Flags.NoInits))
assert(t.constructor.symbol.flags.is(Flags.StableRealizable))

case _ =>
assert(false, s"unexpected ClassDef '${t.name}'")

case _ =>
end inspectClass