Skip to content

Make it possible to always set the prefix of denotations #11330

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 1 commit into from
Feb 7, 2021
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
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,13 @@ object Config {

/** When in IDE, turn StaleSymbol errors into warnings instead of crashing */
inline val ignoreStaleInIDE = true

/** If true, `Denotation#asSeenFrom` is allowed to return an existing
* `SymDenotation` instead of allocating a new `SingleDenotation` if
* the two would only differ in their `prefix` (SymDenotation always
* have `NoPrefix` as their prefix).
* This is done for performance reasons: when compiling Dotty itself this
* reduces the number of allocated denotations by ~50%.
*/
inline val reuseSymDenotations = true
}
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/JavaPlatform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class JavaPlatform extends Platform {
}

// The given symbol is a method with the right name and signature to be a runnable java program.
def isMainMethod(sym: SymDenotation)(using Context): Boolean =
def isMainMethod(sym: Symbol)(using Context): Boolean =
(sym.name == nme.main) && (sym.info match {
case MethodTpe(_, defn.ArrayOf(el) :: Nil, restpe) => el =:= defn.StringType && (restpe isRef defn.UnitClass)
case _ => false
Expand Down
8 changes: 3 additions & 5 deletions compiler/src/dotty/tools/dotc/config/Platform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@ abstract class Platform {
def newClassLoader(bin: AbstractFile)(using Context): SymbolLoader

/** The given symbol is a method with the right name and signature to be a runnable program. */
def isMainMethod(sym: SymDenotation)(using Context): Boolean
def isMainMethod(sym: Symbol)(using Context): Boolean

/** The given class has a main method. */
final def hasMainMethod(sym: Symbol)(using Context): Boolean =
sym.info.member(nme.main).hasAltWith {
case x: SymDenotation => isMainMethod(x) && (sym.is(Module) || x.isStatic)
case _ => false
}
sym.info.member(nme.main).hasAltWith(d =>
isMainMethod(d.symbol) && (sym.is(Module) || d.symbol.isStatic))
}
39 changes: 22 additions & 17 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,12 @@ object Denotations {

final def name(using Context): Name = symbol.name

/** If this is not a SymDenotation: The prefix under which the denotation was constructed.
* NoPrefix for SymDenotations.
/** For SymDenotation, this is NoPrefix. For other denotations this is the prefix
* under which the denotation was constructed.
*
* Note that `asSeenFrom` might return a `SymDenotation` and therefore in
* general one cannot rely on `prefix` being set, see
* `Config.reuseSymDenotations` for details.
*/
def prefix: Type = NoPrefix

Expand Down Expand Up @@ -1044,24 +1048,25 @@ object Denotations {
}

/** The derived denotation with the given `info` transformed with `asSeenFrom`.
* The prefix of the derived denotation is the new prefix `pre` if the type is
* opaque, or if the current prefix is already different from `NoPrefix`.
* That leaves SymDenotations (which have NoPrefix as the prefix), which are left
* as SymDenotations unless the type is opaque. The treatment of opaque types
* is needed, without it i7159.scala fails in from-tasty. Without the treatment,
* opaque type denotations in subclasses are kept as SymDenotations, which means
* that the transform in `ElimOpaque` will return the symbol's opaque alias without
* adding the needed asSeenFrom.
*
* Logically, the right thing to do would be to extend the same treatment to all denotations
* Currently this fails the bootstrap. There's also a concern that this generalization
* would create more denotation objects, at a price in performance.
* As a performance hack, we might reuse an existing SymDenotation,
* instead of creating a new denotation with a given `prefix`,
* see `Config.reuseSymDenotations`.
*/
def derived(info: Type) =
derivedSingleDenotation(
symbol,
info.asSeenFrom(pre, owner),
if (symbol.is(Opaque) || this.prefix != NoPrefix) pre else this.prefix)
/** Do we need to return a denotation with a prefix set? */
def needsPrefix =
// For opaque types, the prefix is used in `ElimOpaques#transform`,
// without this i7159.scala would fail when compiled from tasty.
symbol.is(Opaque)

val derivedInfo = info.asSeenFrom(pre, owner)
if Config.reuseSymDenotations && this.isInstanceOf[SymDenotation]
&& (derivedInfo eq info) && !needsPrefix then
this
else
derivedSingleDenotation(symbol, derivedInfo, pre)
end derived

// Tt could happen that we see the symbol with prefix `this` as a member a different class
// through a self type and that it then has a different info. In this case we have to go
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,8 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisPhase =>
// The Lifter updates the type of symbols using `installAfter` to give them a
// new `SymDenotation`, but that doesn't affect non-sym denotations, so we
// reload them manually here.
// Note: If you tweak this code, make sure to test your changes with
// `Config.reuseSymDenotations` set to false to exercise this path more.
if denot.isInstanceOf[NonSymSingleDenotation] && lifter.free.contains(sym) then
tree.qualifier.select(sym).withSpan(tree.span)
else tree
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/util/Signatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ object Signatures {
case _ =>
val funSymbol = fun.symbol
val alternatives = funSymbol.owner.info.member(funSymbol.name).alternatives
val alternativeIndex = alternatives.indexOf(funSymbol.denot) max 0
val alternativeIndex = alternatives.map(_.symbol).indexOf(funSymbol) max 0
(alternativeIndex, alternatives)
}

Expand Down