Skip to content

Sound type avoidance (hopefully!) #14026

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 11 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fix nesting level to account for block scopes
Previously, the nesting level of a symbol was always equal to the
nesting level of its owner incremented by one, but in a situation like
i8900a.scala where we have:

...
  val a = unwrap[?Outer]({
    class Local
...

the owner of both `?Outer` and `Local` is `a`, and so they ended up with
the same level even though `Local` should not leak outside of the block
that defines it (i8900a.scala compiled regardless due to the disabled
check for forward references in TreePickler re-enabled later in this PR).

We rectify this by associating each scope with a level which is always
greated than the level of the enclosing scope (repurposing the existing
Scope#nestingLevel method which wasn't used for anything), newly created
symbols then simply take the level of the current scope (this required
tweaking typedCase so that the pattern symbols were typed in the scope
were they end up being entered).

Also add a `-Yprint-level` option for debugging level-related issues.
  • Loading branch information
smarter committed Dec 14, 2021
commit eb69870b7170a3db78e1a045c9892a3ba0c27b6a
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
protected def rootContext(using Context): Context = {
ctx.initialize()
ctx.base.setPhasePlan(comp.phases)
val rootScope = new MutableScope
val rootScope = new MutableScope(0)
val bootstrap = ctx.fresh
.setPeriod(Period(comp.nextRunId, FirstPhaseId))
.setScope(rootScope)
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import typer.ProtoTypes
import transform.SymUtils._
import transform.TypeUtils._
import core._
import Scopes.newScope
import util.Spans._, Types._, Contexts._, Constants._, Names._, Flags._, NameOps._
import Symbols._, StdNames._, Annotations._, Trees._, Symbols._
import Decorators._, DenotTransformers._
Expand Down Expand Up @@ -344,7 +345,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
}
else parents
val cls = newNormalizedClassSymbol(owner, tpnme.ANON_CLASS, Synthetic | Final, parents1,
coord = fns.map(_.span).reduceLeft(_ union _))
newScope, coord = fns.map(_.span).reduceLeft(_ union _))
val constr = newConstructor(cls, Synthetic, Nil, Nil).entered
def forwarder(fn: TermSymbol, name: TermName) = {
val fwdMeth = fn.copy(cls, name, Synthetic | Method | Final).entered.asTerm
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ private sealed trait YSettings:
val YprintSyms: Setting[Boolean] = BooleanSetting("-Yprint-syms", "When printing trees print info in symbols instead of corresponding info in trees.")
val YprintDebug: Setting[Boolean] = BooleanSetting("-Yprint-debug", "When printing trees, print some extra information useful for debugging.")
val YprintDebugOwners: Setting[Boolean] = BooleanSetting("-Yprint-debug-owners", "When printing trees, print owners of definitions.")
val YprintLevel: Setting[Boolean] = BooleanSetting("-Yprint-level", "print nesting levels of symbols and type variables.")
val YshowPrintErrors: Setting[Boolean] = BooleanSetting("-Yshow-print-errors", "Don't suppress exceptions thrown during tree printing.")
val YtestPickler: Setting[Boolean] = BooleanSetting("-Ytest-pickler", "Self-test for pickling functionality; should be used with -Ystop-after:pickler.")
val YcheckReentrant: Setting[Boolean] = BooleanSetting("-Ycheck-reentrant", "Check that compiled program does not contain vars that can be accessed from a global root.")
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ object Contexts {
if owner != null && owner.isClass then owner.asClass.unforcedDecls
else scope

def nestingLevel: Int =
val sc = effectiveScope
if sc != null then sc.nestingLevel else 0

/** Sourcefile corresponding to given abstract file, memoized */
def getSource(file: AbstractFile, codec: => Codec = Codec(settings.encoding.value)) = {
util.Stats.record("Context.getSource")
Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ class Definitions {
private def newPermanentClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, infoFn: ClassSymbol => Type) =
newClassSymbol(owner, name, flags | Permanent | NoInits | Open, infoFn)

private def enterCompleteClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, parents: List[TypeRef], decls: Scope = newScope) =
private def enterCompleteClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, parents: List[TypeRef]): ClassSymbol =
enterCompleteClassSymbol(owner, name, flags, parents, newScope(owner.nestingLevel + 1))

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

private def enterTypeField(cls: ClassSymbol, name: TypeName, flags: FlagSet, scope: MutableScope) =
Expand Down Expand Up @@ -433,7 +436,7 @@ class Definitions {
Any_toString, Any_##, Any_getClass, Any_isInstanceOf, Any_typeTest, Object_eq, Object_ne)

@tu lazy val AnyKindClass: ClassSymbol = {
val cls = newCompleteClassSymbol(ScalaPackageClass, tpnme.AnyKind, AbstractFinal | Permanent, Nil)
val cls = newCompleteClassSymbol(ScalaPackageClass, tpnme.AnyKind, AbstractFinal | Permanent, Nil, newScope(0))
if (!ctx.settings.YnoKindPolymorphism.value)
// Enable kind-polymorphism by exposing scala.AnyKind
cls.entered
Expand Down
17 changes: 8 additions & 9 deletions compiler/src/dotty/tools/dotc/core/Scopes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ object Scopes {
*/
def size: Int

/** The number of outer scopes from which symbols are inherited */
/** The number of scopes enclosing this scope. */
def nestingLevel: Int

/** The symbols in this scope in the order they were entered;
Expand Down Expand Up @@ -193,15 +193,15 @@ object Scopes {
* This is necessary because when run from reflection every scope needs to have a
* SynchronizedScope as mixin.
*/
class MutableScope protected[Scopes](initElems: ScopeEntry, initSize: Int, val nestingLevel: Int = 0)
class MutableScope protected[Scopes](initElems: ScopeEntry, initSize: Int, val nestingLevel: Int)
extends Scope {

/** Scope shares elements with `base` */
protected[Scopes] def this(base: Scope)(using Context) =
this(base.lastEntry, base.size, base.nestingLevel + 1)
ensureCapacity(MinHashedScopeSize)

def this() = this(null, 0, 0)
def this(nestingLevel: Int) = this(null, 0, nestingLevel)

private[dotc] var lastEntry: ScopeEntry = initElems

Expand All @@ -225,7 +225,7 @@ object Scopes {
/** Use specified synthesize for this scope */
def useSynthesizer(s: SymbolSynthesizer): Unit = synthesize = s

protected def newScopeLikeThis(): MutableScope = new MutableScope()
protected def newScopeLikeThis(): MutableScope = new MutableScope(nestingLevel)

/** Clone scope, taking care not to force the denotations of any symbols in the scope.
*/
Expand Down Expand Up @@ -440,7 +440,10 @@ object Scopes {
}

/** Create a new scope */
def newScope: MutableScope = new MutableScope()
def newScope(using Context): MutableScope =
new MutableScope(ctx.nestingLevel + 1)

def newScope(nestingLevel: Int): MutableScope = new MutableScope(nestingLevel)

/** Create a new scope nested in another one with which it shares its elements */
def newNestedScope(outer: Scope)(using Context): MutableScope = new MutableScope(outer)
Expand Down Expand Up @@ -468,8 +471,4 @@ object Scopes {
override def lookupEntry(name: Name)(using Context): ScopeEntry = null
override def lookupNextEntry(entry: ScopeEntry)(using Context): ScopeEntry = null
}

/** A class for error scopes (mutable)
*/
class ErrorScope(owner: Symbol) extends MutableScope
}
14 changes: 0 additions & 14 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1477,14 +1477,6 @@ object SymDenotations {
else if is(Contravariant) then Contravariant
else EmptyFlags

/** The length of the owner chain of this symbol. 1 for _root_, 0 for NoSymbol */
def nestingLevel(using Context): Int =
@tailrec def recur(d: SymDenotation, n: Int): Int = d match
case NoDenotation => n
case d: ClassDenotation => d.nestingLevel + n // profit from the cache in ClassDenotation
case _ => recur(d.owner, n + 1)
recur(this, 0)

/** The flags to be used for a type parameter owned by this symbol.
* Overridden by ClassDenotation.
*/
Expand Down Expand Up @@ -2323,12 +2315,6 @@ object SymDenotations {

override def registeredCompanion_=(c: Symbol) =
myCompanion = c

private var myNestingLevel = -1

override def nestingLevel(using Context) =
if myNestingLevel == -1 then myNestingLevel = owner.nestingLevel + 1
myNestingLevel
}

/** The denotation of a package class.
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ object SymbolLoaders {
/** The scope of a package. This is different from a normal scope
* in that names of scope entries are kept in mangled form.
*/
final class PackageScope extends MutableScope {
final class PackageScope extends MutableScope(0) {
override def newScopeEntry(name: Name, sym: Symbol)(using Context): ScopeEntry =
super.newScopeEntry(name.mangled, sym)

Expand Down
20 changes: 10 additions & 10 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ object Symbols {
* @param coord The coordinates of the symbol (a position or an index)
* @param id A unique identifier of the symbol (unique per ContextBase)
*/
class Symbol private[Symbols] (private var myCoord: Coord, val id: Int)
class Symbol private[Symbols] (private var myCoord: Coord, val id: Int, val nestingLevel: Int)
extends Designator, ParamInfo, SrcPos, printing.Showable {

type ThisName <: Name
Expand Down Expand Up @@ -368,8 +368,8 @@ object Symbols {
type TermSymbol = Symbol { type ThisName = TermName }
type TypeSymbol = Symbol { type ThisName = TypeName }

class ClassSymbol private[Symbols] (coord: Coord, val assocFile: AbstractFile, id: Int)
extends Symbol(coord, id) {
class ClassSymbol private[Symbols] (coord: Coord, val assocFile: AbstractFile, id: Int, nestingLevel: Int)
extends Symbol(coord, id, nestingLevel) {

type ThisName = TypeName

Expand Down Expand Up @@ -459,7 +459,7 @@ object Symbols {
override protected def prefixString: String = "ClassSymbol"
}

@sharable object NoSymbol extends Symbol(NoCoord, 0) {
@sharable object NoSymbol extends Symbol(NoCoord, 0, 0) {
override def associatedFile(using Context): AbstractFile = NoSource.file
override def recomputeDenot(lastd: SymDenotation)(using Context): SymDenotation = NoDenotation
}
Expand Down Expand Up @@ -516,7 +516,7 @@ object Symbols {
info: Type,
privateWithin: Symbol = NoSymbol,
coord: Coord = NoCoord)(using Context): Symbol { type ThisName = N } = {
val sym = new Symbol(coord, ctx.base.nextSymId).asInstanceOf[Symbol { type ThisName = N }]
val sym = new Symbol(coord, ctx.base.nextSymId, ctx.nestingLevel).asInstanceOf[Symbol { type ThisName = N }]
val denot = SymDenotation(sym, owner, name, flags, info, privateWithin)
sym.denot = denot
sym
Expand All @@ -534,7 +534,7 @@ object Symbols {
coord: Coord = NoCoord,
assocFile: AbstractFile = null)(using Context): ClassSymbol
= {
val cls = new ClassSymbol(coord, assocFile, ctx.base.nextSymId)
val cls = new ClassSymbol(coord, assocFile, ctx.base.nextSymId, ctx.nestingLevel)
val denot = SymDenotation(cls, owner, name, flags, infoFn(cls), privateWithin)
cls.denot = denot
cls
Expand All @@ -546,7 +546,7 @@ object Symbols {
name: TypeName,
flags: FlagSet,
parents: List[TypeRef],
decls: Scope = newScope,
decls: Scope,
selfInfo: Type = NoType,
privateWithin: Symbol = NoSymbol,
coord: Coord = NoCoord,
Expand All @@ -564,7 +564,7 @@ object Symbols {
name: TypeName,
flags: FlagSet,
parentTypes: List[Type],
decls: Scope = newScope,
decls: Scope,
selfInfo: Type = NoType,
privateWithin: Symbol = NoSymbol,
coord: Coord = NoCoord,
Expand All @@ -580,7 +580,7 @@ object Symbols {
}

def newRefinedClassSymbol(coord: Coord = NoCoord)(using Context): ClassSymbol =
newCompleteClassSymbol(ctx.owner, tpnme.REFINE_CLASS, NonMember, parents = Nil, coord = coord)
newCompleteClassSymbol(ctx.owner, tpnme.REFINE_CLASS, NonMember, parents = Nil, newScope, coord = coord)

/** Create a module symbol with associated module class
* from its non-info fields and a function producing the info
Expand Down Expand Up @@ -646,7 +646,7 @@ object Symbols {
name: TermName,
modFlags: FlagSet = EmptyFlags,
clsFlags: FlagSet = EmptyFlags,
decls: Scope = newScope)(using Context): TermSymbol =
decls: Scope = newScope(0))(using Context): TermSymbol =
newCompleteModuleSymbol(
owner, name,
modFlags | PackageCreationFlags, clsFlags | PackageCreationFlags,
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4667,7 +4667,7 @@ object Types {
* @param origin The parameter that's tracked by the type variable.
* @param creatorState The typer state in which the variable was created.
*/
final class TypeVar private(initOrigin: TypeParamRef, creatorState: TyperState, nestingLevel: Int) extends CachedProxyType with ValueType {
final class TypeVar private(initOrigin: TypeParamRef, creatorState: TyperState, val nestingLevel: Int) extends CachedProxyType with ValueType {

private var currentOrigin = initOrigin

Expand Down Expand Up @@ -4713,6 +4713,8 @@ object Types {
* are nested more deeply than the type variable itself.
*/
private def avoidCaptures(tp: Type)(using Context): Type =
if ctx.isAfterTyper then
return tp
val problemSyms = new TypeAccumulator[Set[Symbol]]:
def apply(syms: Set[Symbol], t: Type): Set[Symbol] = t match
case ref: NamedType
Expand Down Expand Up @@ -4806,7 +4808,7 @@ object Types {
}
object TypeVar:
def apply(initOrigin: TypeParamRef, creatorState: TyperState)(using Context) =
new TypeVar(initOrigin, creatorState, ctx.owner.nestingLevel)
new TypeVar(initOrigin, creatorState, ctx.nestingLevel)

type TypeVars = SimpleIdentitySet[TypeVar]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class ClassfileParser(

protected val staticModule: Symbol = moduleRoot.sourceModule(using ictx)

protected val instanceScope: MutableScope = newScope // the scope of all instance definitions
protected val staticScope: MutableScope = newScope // the scope of all static definitions
protected val instanceScope: MutableScope = newScope(0) // the scope of all instance definitions
protected val staticScope: MutableScope = newScope(0) // the scope of all static definitions
protected var pool: ConstantPool = _ // the classfile's constant pool

protected var currentClassName: SimpleName = _ // JVM name of the current class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
}

/** The `decls` scope associated with given symbol */
protected def symScope(sym: Symbol): Scope = symScopes.getOrElseUpdate(sym, newScope)
protected def symScope(sym: Symbol): Scope = symScopes.getOrElseUpdate(sym, newScope(0))

/** Does entry represent an (internal) symbol */
protected def isSymbolEntry(i: Int)(using Context): Boolean = {
Expand Down
16 changes: 13 additions & 3 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
protected def maxToTextRecursions: Int = 100

protected def showUniqueIds = ctx.settings.uniqid.value || Printer.debugPrintUnique
protected def showNestingLevel = ctx.settings.YprintLevel.value

protected final def limiter: MessageLimiter = ctx.property(MessageLimiter).get

Expand Down Expand Up @@ -155,7 +156,12 @@ class PlainPrinter(_ctx: Context) extends Printer {
case tp: TermParamRef =>
ParamRefNameString(tp) ~ lambdaHash(tp.binder) ~ ".type"
case tp: TypeParamRef =>
ParamRefNameString(tp) ~ lambdaHash(tp.binder)
val suffix =
if showNestingLevel then
val tvar = ctx.typerState.constraint.typeVarOfParam(tp)
if tvar.exists then s"#${tvar.asInstanceOf[TypeVar].nestingLevel.toString}" else ""
else ""
ParamRefNameString(tp) ~ lambdaHash(tp.binder) ~ suffix
case tp: SingletonType =>
toTextSingleton(tp)
case AppliedType(tycon, args) =>
Expand Down Expand Up @@ -271,9 +277,13 @@ class PlainPrinter(_ctx: Context) extends Printer {
catch { case ex: NullPointerException => "" }
else ""

/** If -uniqid is set, the unique id of symbol, after a # */
/** A string to append to a symbol composed of:
* - if -uniqid is set, its unique id after a #.
* - if -Yprint-level, its nesting level after a %.
*/
protected def idString(sym: Symbol): String =
if (showUniqueIds || Printer.debugPrintUnique) "#" + sym.id else ""
(if (showUniqueIds || Printer.debugPrintUnique) "#" + sym.id else "") +
(if (showNestingLevel) "%" + sym.nestingLevel else "")

def nameString(sym: Symbol): String =
simpleNameString(sym) + idString(sym) // + "<" + (if (sym.exists) sym.owner else "") + ">"
Expand Down
10 changes: 9 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,15 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
protected def optAscription[T >: Untyped](tpt: Tree[T]): Text = optText(tpt)(": " ~ _)

private def idText(tree: untpd.Tree): Text =
if showUniqueIds && tree.hasType && tree.symbol.exists then s"#${tree.symbol.id}" else ""
(if showUniqueIds && tree.hasType && tree.symbol.exists then s"#${tree.symbol.id}" else "") ~
(if showNestingLevel then tree.typeOpt match
case tp: NamedType if !tp.symbol.isStatic => s"%${tp.symbol.nestingLevel}"
case tp: TypeVar => s"%${tp.nestingLevel}"
case tp: TypeParamRef => ctx.typerState.constraint.typeVarOfParam(tp) match
case tvar: TypeVar => s"%${tvar.nestingLevel}"
case _ => ""
case _ => ""
else "")

private def useSymbol(tree: untpd.Tree) =
tree.hasType && tree.symbol.exists && ctx.settings.YprintSyms.value
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/quoted/MacroExpansion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ object MacroExpansion {
ctx.property(MacroExpansionPosition)

def context(inlinedFrom: tpd.Tree)(using Context): Context =
QuotesCache.init(ctx.fresh).setProperty(MacroExpansionPosition, SourcePosition(inlinedFrom.source, inlinedFrom.span)).setTypeAssigner(new Typer).withSource(inlinedFrom.source)
QuotesCache.init(ctx.fresh).setProperty(MacroExpansionPosition, SourcePosition(inlinedFrom.source, inlinedFrom.span)).setTypeAssigner(new Typer(ctx.nestingLevel + 1)).withSource(inlinedFrom.source)
}

3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dotc
package transform

import core._
import Scopes.newScope
import Contexts._, Symbols._, Types._, Flags._, Decorators._, StdNames._, Constants._
import MegaPhase._
import SymUtils._
Expand Down Expand Up @@ -123,7 +124,7 @@ class ExpandSAMs extends MiniPhase:
val parents = List(
defn.AbstractPartialFunctionClass.typeRef.appliedTo(anonTpe.firstParamTypes.head, anonTpe.resultType),
defn.SerializableType)
val pfSym = newNormalizedClassSymbol(anonSym.owner, tpnme.ANON_CLASS, Synthetic | Final, parents, coord = tree.span)
val pfSym = newNormalizedClassSymbol(anonSym.owner, tpnme.ANON_CLASS, Synthetic | Final, parents, newScope, coord = tree.span)

def overrideSym(sym: Symbol) = sym.copy(
owner = pfSym,
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Constants._
import ProtoTypes._
import ErrorReporting._
import Inferencing.{fullyDefinedType, isFullyDefined}
import Scopes.newScope
import Trees._
import transform.SymUtils._
import transform.TypeUtils._
Expand Down Expand Up @@ -1774,7 +1775,7 @@ final class SearchRoot extends SearchHistory:
// }

val parents = List(defn.ObjectType, defn.SerializableType)
val classSym = newNormalizedClassSymbol(ctx.owner, LazyImplicitName.fresh().toTypeName, Synthetic | Final, parents, coord = span)
val classSym = newNormalizedClassSymbol(ctx.owner, LazyImplicitName.fresh().toTypeName, Synthetic | Final, parents, newScope, coord = span)
val vsyms = pruned.map(_._1.symbol)
val nsyms = vsyms.map(vsym => newSymbol(classSym, vsym.name, EmptyFlags, vsym.info, coord = span).entered)
val vsymMap = (vsyms zip nsyms).toMap
Expand Down
Loading