Skip to content

Some Rudimentary Tracking of Class Purity #4710

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 7 commits into from
Jul 8, 2018
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
66 changes: 45 additions & 21 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,35 @@ trait TreeInfo[T >: Untyped <: Type] { self: Trees.Instance[T] =>
case y => y
}

/** The largest subset of {NoInits, PureInterface} that a
* trait or class enclosing this statement can have as flags.
*/
def defKind(tree: Tree)(implicit ctx: Context): FlagSet = unsplice(tree) match {
case EmptyTree | _: Import => NoInitsInterface
case tree: TypeDef => if (tree.isClassDef) NoInits else NoInitsInterface
case tree: DefDef =>
if (tree.unforcedRhs == EmptyTree &&
tree.vparamss.forall(_.forall(_.rhs.isEmpty))) NoInitsInterface
else NoInits
case tree: ValDef => if (tree.unforcedRhs == EmptyTree) NoInitsInterface else EmptyFlags
case _ => EmptyFlags
}

/** The largest subset of {NoInits, PureInterface} that a
* trait or class with these parents can have as flags.
*/
def parentsKind(parents: List[Tree])(implicit ctx: Context): FlagSet = parents match {
case Nil => NoInitsInterface
case Apply(_, _ :: _) :: _ => EmptyFlags
case _ :: parents1 => parentsKind(parents1)
}

/** The largest subset of {NoInits, PureInterface} that a
* trait or class with this body can have as flags.
*/
def bodyKind(body: List[Tree])(implicit ctx: Context): FlagSet =
(NoInitsInterface /: body)((fs, stat) => fs & defKind(stat))

/** Checks whether predicate `p` is true for all result parts of this expression,
* where we zoom into Ifs, Matches, and Blocks.
*/
Expand Down Expand Up @@ -358,6 +387,8 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
refPurity(tree)
case Select(qual, _) =>
refPurity(tree).min(exprPurity(qual))
case New(_) =>
SimplyPure
case TypeApply(fn, _) =>
exprPurity(fn)
/*
Expand All @@ -369,13 +400,12 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
case Apply(fn, args) =>
def isKnownPureOp(sym: Symbol) =
sym.owner.isPrimitiveValueClass || sym.owner == defn.StringClass
// Note: After uncurry, field accesses are represented as Apply(getter, Nil),
// so an Apply can also be pure.
if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn)
else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol))
// A constant expression with pure arguments is pure.
if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol)
// A constant expression with pure arguments is pure.
|| fn.symbol.isStable)
minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
else Impure
else
Impure
case Typed(expr, _) =>
exprPurity(expr)
case Block(stats, expr) =>
Expand All @@ -402,11 +432,16 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
* @DarkDimius: need to make sure that lazy accessor methods have Lazy and Stable
* flags set.
*/
def refPurity(tree: Tree)(implicit ctx: Context): PurityLevel =
if (!tree.tpe.widen.isParameterless || tree.symbol.is(Erased)) SimplyPure
else if (!tree.symbol.isStable) Impure
else if (tree.symbol.is(Lazy)) Idempotent // TODO add Module flag, sinxce Module vals or not Lazy from the start.
def refPurity(tree: Tree)(implicit ctx: Context): PurityLevel = {
val sym = tree.symbol
if (!tree.hasType) Impure
else if (!tree.tpe.widen.isParameterless || sym.is(Erased)) SimplyPure
else if (!sym.isStable) Impure
else if (sym.is(Module))
if (sym.moduleClass.isNoInitsClass) Pure else Idempotent
else if (sym.is(Lazy)) Idempotent
else SimplyPure
}

def isPureRef(tree: Tree)(implicit ctx: Context) =
refPurity(tree) == SimplyPure
Expand Down Expand Up @@ -623,17 +658,6 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
accum(Nil, root)
}

/** The largest subset of {NoInits, PureInterface} that a
* trait enclosing this statement can have as flags.
*/
def defKind(tree: Tree): FlagSet = unsplice(tree) match {
case EmptyTree | _: Import => NoInitsInterface
case tree: TypeDef => if (tree.isClassDef) NoInits else NoInitsInterface
case tree: DefDef => if (tree.unforcedRhs == EmptyTree) NoInitsInterface else NoInits
case tree: ValDef => if (tree.unforcedRhs == EmptyTree) NoInitsInterface else EmptyFlags
case _ => EmptyFlags
}

/** The top level classes in this tree, including only those module classes that
* are not a linked class of some other class in the result.
*/
Expand Down
12 changes: 10 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ class Definitions {
ctx.newSymbol(owner, name, flags | Permanent, info)

private def newClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, infoFn: ClassSymbol => Type) =
ctx.newClassSymbol(owner, name, flags | Permanent, infoFn)
ctx.newClassSymbol(owner, name, flags | Permanent | NoInits, infoFn)

private def enterCompleteClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, parents: List[TypeRef], decls: Scope = newScope) =
ctx.newCompleteClassSymbol(owner, name, flags | Permanent, parents, decls).entered
ctx.newCompleteClassSymbol(owner, name, flags | Permanent | NoInits, parents, decls).entered

private def enterTypeField(cls: ClassSymbol, name: TypeName, flags: FlagSet, scope: MutableScope) =
scope.enter(newSymbol(cls, name, flags, TypeBounds.empty))
Expand Down Expand Up @@ -275,6 +275,7 @@ class Definitions {
val cls = ctx.requiredClass("java.lang.Object")
assert(!cls.isCompleted, "race for completing java.lang.Object")
cls.info = ClassInfo(cls.owner.thisType, cls, AnyClass.typeRef :: Nil, newScope)
cls.setFlag(NoInits)

// The companion object doesn't really exist, `NoType` is the general
// technique to do that. Here we need to set it before completing
Expand Down Expand Up @@ -1215,6 +1216,13 @@ class Definitions {
for (m <- ScalaShadowingPackageClass.info.decls)
ScalaPackageClass.enter(m)

// 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
// requiers that we read all Scala code from Tasty.
ProductClass.setFlag(NoInits)
SerializableClass.setFlag(NoInits)

// force initialization of every symbol that is synthesized or hijacked by the compiler
val forced = syntheticCoreClasses ++ syntheticCoreMethods ++ ScalaValueClasses()

Expand Down
19 changes: 14 additions & 5 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,14 @@ object SymDenotations {
/** Unset given flags(s) of this denotation */
final def resetFlag(flags: FlagSet): Unit = { myFlags &~= flags }

/** Set applicable flags from `flags` which is a subset of {NoInits, PureInterface} */
final def setNoInitsFlags(flags: FlagSet): Unit = {
val mask = if (myFlags.is(Trait)) NoInitsInterface else NoInits
setFlag(flags & mask)
}
/** Set applicable flags in {NoInits, PureInterface}
* @param parentFlags The flags that match the class or trait's parents
* @param bodyFlags The flags that match the class or trait's body
*/
final def setNoInitsFlags(parentFlags: FlagSet, bodyFlags: FlagSet): Unit =
setFlag(
if (myFlags.is(Trait)) NoInitsInterface & bodyFlags // no parents are initialized from a trait
else NoInits & bodyFlags & parentFlags)

private def isCurrent(fs: FlagSet) =
fs <= (
Expand Down Expand Up @@ -599,6 +602,12 @@ object SymDenotations {
final def isStable(implicit ctx: Context) =
isType || !is(Erased) && (is(Stable) || !(is(UnstableValue) || info.isInstanceOf[ExprType]))

/** Is this a denotation of a class that does not have - either direct or inherited -
* initaliazion code?
*/
def isNoInitsClass(implicit ctx: Context) =
isClass && asClass.baseClasses.forall(_.is(NoInits))

/** Is this a "real" method? A real method is a method which is:
* - not an accessor
* - not a label
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ class TreeUnpickler(reader: TastyReader,
else EmptyValDef
cls.info = ClassInfo(cls.owner.thisType, cls, parentTypes, cls.unforcedDecls,
if (self.isEmpty) NoType else self.tpt.tpe)
cls.setNoInitsFlags(fork.indexStats(end))
cls.setNoInitsFlags(parentsKind(parents), fork.indexStats(end))
val constr = readIndexedDef().asInstanceOf[DefDef]
val mappedParents = parents.map(_.changeOwner(localDummy, constr.symbol))

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ private class ExtractAPICollector(implicit val ctx: Context) extends ThunkHolder
} else if (sym.is(Mutable, butNot = Accessor)) {
api.Var.of(sym.name.toString, apiAccess(sym), apiModifiers(sym),
apiAnnotations(sym).toArray, apiType(sym.info))
} else if (sym.isStable) {
} else if (sym.isStable && !sym.isRealMethod) {
api.Val.of(sym.name.toString, apiAccess(sym), apiModifiers(sym),
apiAnnotations(sym).toArray, apiType(sym.info))
} else {
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,8 @@ class Namer { typer: Typer =>
if (isDerivedValueClass(cls)) cls.setFlag(Final)
cls.info = avoidPrivateLeaks(cls, cls.pos)
cls.baseClasses.foreach(_.invalidateBaseTypeCache()) // we might have looked before and found nothing
cls.setNoInitsFlags(parentsKind(parents), bodyKind(rest))
if (cls.isNoInitsClass) cls.primaryConstructor.setFlag(Stable)
}
}

Expand Down
5 changes: 2 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1534,8 +1534,6 @@ class Typer extends Namer
val dummy = localDummy(cls, impl)
val body1 = addAccessorDefs(cls,
typedStats(impl.body, dummy)(ctx.inClassContext(self1.symbol)))
if (!ctx.isAfterTyper)
cls.setNoInitsFlags((NoInitsInterface /: body1) ((fs, stat) => fs & defKind(stat)))

// Expand comments and type usecases if `-Ycook-comments` is set.
if (ctx.settings.YcookComments.value) {
Expand Down Expand Up @@ -1919,7 +1917,8 @@ class Typer extends Namer
traverse(stats ++ rest)
case stat :: rest =>
val stat1 = typed(stat)(ctx.exprContext(stat, exprOwner))
if (!ctx.isAfterTyper && isPureExpr(stat1) && !stat1.tpe.isRef(defn.UnitClass))
if (!ctx.isAfterTyper && isPureExpr(stat1) &&
!stat1.tpe.isRef(defn.UnitClass) && !isSelfOrSuperConstrCall(stat1))
ctx.warning(em"a pure expression does nothing in statement position", stat.pos)
buf += stat1
traverse(rest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
"""
|object Scope {
| abstract class Concept
| new Concept()
| val x = new Concept()
|}
""".stripMargin
}
Expand All @@ -181,7 +181,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
"""
|object Scope {
| trait Concept
| new Concept()
| val x = new Concept()
|}
""".stripMargin
}
Expand Down Expand Up @@ -508,7 +508,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
"""class Base
|class RequiresBase { self: Base => }
|object Scope {
| new RequiresBase
| val x = new RequiresBase
|}
|""".stripMargin
}
Expand Down
1 change: 1 addition & 0 deletions tests/neg-custom-args/fatal-warnings/i2333.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@deprecated("bla", "2.11.0") class Foo {
println("")
def this(x: Int) = this()
}

Expand Down