From 441ba4e64c40bb636bdacb942b41f73ff67e9bbc Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 17 Nov 2021 11:21:26 +0000 Subject: [PATCH 1/2] Replace Set with UniqList in memberNames --- .../tools/backend/jvm/BCodeHelpers.scala | 9 +++----- .../tools/dotc/core/SymDenotations.scala | 19 +++++++++-------- .../src/dotty/tools/dotc/core/Types.scala | 5 +++-- .../dotty/tools/dotc/typer/RefChecks.scala | 4 ++-- .../src/dotty/tools/dotc/util/UniqList.scala | 21 +++++++++++++++++++ 5 files changed, 39 insertions(+), 19 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/util/UniqList.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index 03d5ac13665b..75391ae5c23f 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -615,14 +615,11 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { * The members are sorted by name and signature to guarantee a stable ordering. */ private def sortedMembersBasedOnFlags(tp: Type, required: Flag, excluded: FlagSet): List[Symbol] = { - // The output of `memberNames` is a Set, sort it to guarantee a stable ordering. - val names = tp.memberNames(takeAllFilter).toSeq.sorted - val buffer = mutable.ListBuffer[Symbol]() - names.foreach { name => - buffer ++= tp.memberBasedOnFlags(name, required, excluded) + val names = tp.memberNames(takeAllFilter).toList + names.flatMap { name => + tp.memberBasedOnFlags(name, required, excluded) .alternatives.sortBy(_.signature)(Signature.lexicographicOrdering).map(_.symbol) } - buffer.toList } /* diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 7f47f1696cd6..28170585ffd0 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -18,6 +18,7 @@ import Variances.Variance import annotation.tailrec import util.SimpleIdentityMap import util.Stats +import util.UniqList import java.util.WeakHashMap import scala.util.control.NonFatal import config.Config @@ -2231,7 +2232,7 @@ object SymDenotations { } } - def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = + def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = if (this.is(PackageClass) || !Config.cacheMemberNames) computeMemberNames(keepOnly) // don't cache package member names; they might change else { @@ -2239,8 +2240,8 @@ object SymDenotations { memberNamesCache(keepOnly, this) } - def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = { - var names = Set[Name]() + def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = { + val names = new UniqList.Builder[Name] def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name try { for (p <- parentSyms if p.isClass) @@ -2253,7 +2254,7 @@ object SymDenotations { else info.decls.iterator.filter(_.isOneOf(GivenOrImplicitVal)) else info.decls.iterator for (sym <- ownSyms) maybeAdd(sym.name) - names + names.result } catch { case ex: Throwable => @@ -2466,10 +2467,10 @@ object SymDenotations { end computeMembersNamed /** The union of the member names of the package and the package object */ - override def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = { - def recur(pobjs: List[ClassDenotation], acc: Set[Name]): Set[Name] = pobjs match { + override def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = { + def recur(pobjs: List[ClassDenotation], acc: UniqList[Name]): UniqList[Name] = pobjs match { case pcls :: pobjs1 => - recur(pobjs1, acc.union(pcls.memberNames(keepOnly))) + recur(pobjs1, acc | pcls.memberNames(keepOnly)) case nil => acc } @@ -2752,7 +2753,7 @@ object SymDenotations { /** A cache for sets of member names, indexed by a NameFilter */ trait MemberNames extends InheritedCache { def apply(keepOnly: NameFilter, clsd: ClassDenotation) - (implicit onBehalf: MemberNames, ctx: Context): Set[Name] + (implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] } object MemberNames { @@ -2813,7 +2814,7 @@ object SymDenotations { } private class MemberNamesImpl(createdAt: Period) extends InheritedCacheImpl(createdAt) with MemberNames { - private var cache: SimpleIdentityMap[NameFilter, Set[Name]] = SimpleIdentityMap.empty + private var cache: SimpleIdentityMap[NameFilter, UniqList[Name]] = SimpleIdentityMap.empty final def isValid(using Context): Boolean = cache != null && isValidAt(ctx.phase) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b384cbdcb084..b67fd02bd9bd 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -23,6 +23,7 @@ import Variances.{Variance, varianceFromInt, varianceToInt, setStructuralVarianc import typer.Nullables import util.Stats._ import util.SimpleIdentitySet +import util.UniqList import ast.tpd._ import ast.TreeTypeMap import printing.Texts._ @@ -902,7 +903,7 @@ object Types { * @note: OK to use a Set[Name] here because Name hashcodes are replayable, * hence the Set will always give the same names in the same order. */ - final def memberNames(keepOnly: NameFilter, pre: Type = this)(using Context): Set[Name] = this match { + final def memberNames(keepOnly: NameFilter, pre: Type = this)(using Context): UniqList[Name] = this match { case tp: ClassInfo => val names = tp.cls.classDenot.memberNames(keepOnly) if keepOnly.isStable then names else names.filter(keepOnly(pre, _)) @@ -916,7 +917,7 @@ object Types { case tp: OrType => tp.tp1.memberNames(keepOnly, pre) & tp.tp2.memberNames(keepOnly, pre) case _ => - Set() + UniqList.empty } def memberDenots(keepOnly: NameFilter, f: (Name, mutable.Buffer[SingleDenotation]) => Unit)(using Context): Seq[SingleDenotation] = { diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 4c20f5821ca4..34cf91c73d88 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -51,9 +51,9 @@ object RefChecks { } { val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter) - val defaultMethodNames = defaultGetterNames map { _ replace { + val defaultMethodNames = defaultGetterNames.toList.map { _ replace { case DefaultGetterName(methName, _) => methName - }} + }}.distinct for (name <- defaultMethodNames) { val methods = clazz.info.member(name).alternatives.map(_.symbol) diff --git a/compiler/src/dotty/tools/dotc/util/UniqList.scala b/compiler/src/dotty/tools/dotc/util/UniqList.scala new file mode 100644 index 000000000000..1c7a45821b36 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/util/UniqList.scala @@ -0,0 +1,21 @@ +package dotty.tools.dotc.util + +opaque type UniqList[+A] >: Null <: AnyRef = List[A] + +object UniqList: + def empty: UniqList[Nothing] = Nil + extension [A](xs: UniqList[A]) + def foreach(f: A => Unit): Unit = xs.foreach(f) + def filter(p: A => Boolean): UniqList[A] = xs.filter(p) + def + (x: A): UniqList[A] = if xs.contains(x) then xs else xs :+ x + def & (ys: UniqList[A]): UniqList[A] = xs.filter(ys.contains) + def | (ys: UniqList[A]): UniqList[A] = (xs ::: ys).distinct + def toList: List[A] = xs + + class Builder[A] extends scala.collection.mutable.Builder[A, UniqList[A]]: + private val xs = new scala.collection.mutable.ListBuffer[A] + override def addOne(x: A): this.type = { xs += x; this } + override def addAll(ys: IterableOnce[A]): this.type = { xs ++= ys; this } + override def clear(): Unit = xs.clear() + override def result(): UniqList[A] = xs.distinct.toList + override def sizeHint(size: Int) = xs.sizeHint(size) From ef84745be8ddae34a21115d185dccd6f30671d97 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Sun, 28 Nov 2021 18:22:30 +0000 Subject: [PATCH 2/2] Use the standard immutable SortedSet instead --- .../tools/dotc/core/SymDenotations.scala | 20 +++++++++--------- .../src/dotty/tools/dotc/core/Types.scala | 6 +++--- .../dotty/tools/dotc/typer/RefChecks.scala | 4 ++-- .../src/dotty/tools/dotc/util/UniqList.scala | 21 ------------------- 4 files changed, 15 insertions(+), 36 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/util/UniqList.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 28170585ffd0..52c99122fafa 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -18,11 +18,11 @@ import Variances.Variance import annotation.tailrec import util.SimpleIdentityMap import util.Stats -import util.UniqList import java.util.WeakHashMap import scala.util.control.NonFatal import config.Config import reporting._ +import collection.immutable.SortedSet import collection.mutable import transform.TypeUtils._ @@ -2232,7 +2232,7 @@ object SymDenotations { } } - def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = + def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): SortedSet[Name] = if (this.is(PackageClass) || !Config.cacheMemberNames) computeMemberNames(keepOnly) // don't cache package member names; they might change else { @@ -2240,8 +2240,8 @@ object SymDenotations { memberNamesCache(keepOnly, this) } - def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = { - val names = new UniqList.Builder[Name] + def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): SortedSet[Name] = { + var names = SortedSet[Name]() def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name try { for (p <- parentSyms if p.isClass) @@ -2254,7 +2254,7 @@ object SymDenotations { else info.decls.iterator.filter(_.isOneOf(GivenOrImplicitVal)) else info.decls.iterator for (sym <- ownSyms) maybeAdd(sym.name) - names.result + names } catch { case ex: Throwable => @@ -2467,10 +2467,10 @@ object SymDenotations { end computeMembersNamed /** The union of the member names of the package and the package object */ - override def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] = { - def recur(pobjs: List[ClassDenotation], acc: UniqList[Name]): UniqList[Name] = pobjs match { + override def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): SortedSet[Name] = { + def recur(pobjs: List[ClassDenotation], acc: SortedSet[Name]): SortedSet[Name] = pobjs match { case pcls :: pobjs1 => - recur(pobjs1, acc | pcls.memberNames(keepOnly)) + recur(pobjs1, acc.union(pcls.memberNames(keepOnly))) case nil => acc } @@ -2753,7 +2753,7 @@ object SymDenotations { /** A cache for sets of member names, indexed by a NameFilter */ trait MemberNames extends InheritedCache { def apply(keepOnly: NameFilter, clsd: ClassDenotation) - (implicit onBehalf: MemberNames, ctx: Context): UniqList[Name] + (implicit onBehalf: MemberNames, ctx: Context): SortedSet[Name] } object MemberNames { @@ -2814,7 +2814,7 @@ object SymDenotations { } private class MemberNamesImpl(createdAt: Period) extends InheritedCacheImpl(createdAt) with MemberNames { - private var cache: SimpleIdentityMap[NameFilter, UniqList[Name]] = SimpleIdentityMap.empty + private var cache: SimpleIdentityMap[NameFilter, SortedSet[Name]] = SimpleIdentityMap.empty final def isValid(using Context): Boolean = cache != null && isValidAt(ctx.phase) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b67fd02bd9bd..fe0f309029ed 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -23,13 +23,13 @@ import Variances.{Variance, varianceFromInt, varianceToInt, setStructuralVarianc import typer.Nullables import util.Stats._ import util.SimpleIdentitySet -import util.UniqList import ast.tpd._ import ast.TreeTypeMap import printing.Texts._ import printing.Printer import Hashable._ import Uniques._ +import collection.immutable.SortedSet import collection.mutable import config.Config import config.Feature @@ -903,7 +903,7 @@ object Types { * @note: OK to use a Set[Name] here because Name hashcodes are replayable, * hence the Set will always give the same names in the same order. */ - final def memberNames(keepOnly: NameFilter, pre: Type = this)(using Context): UniqList[Name] = this match { + final def memberNames(keepOnly: NameFilter, pre: Type = this)(using Context): SortedSet[Name] = this match { case tp: ClassInfo => val names = tp.cls.classDenot.memberNames(keepOnly) if keepOnly.isStable then names else names.filter(keepOnly(pre, _)) @@ -917,7 +917,7 @@ object Types { case tp: OrType => tp.tp1.memberNames(keepOnly, pre) & tp.tp2.memberNames(keepOnly, pre) case _ => - UniqList.empty + SortedSet() } def memberDenots(keepOnly: NameFilter, f: (Name, mutable.Buffer[SingleDenotation]) => Unit)(using Context): Seq[SingleDenotation] = { diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 34cf91c73d88..f3866f0b0f41 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -51,9 +51,9 @@ object RefChecks { } { val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter) - val defaultMethodNames = defaultGetterNames.toList.map { _ replace { + val defaultMethodNames = defaultGetterNames.map { _ replace { case DefaultGetterName(methName, _) => methName - }}.distinct + }}(Names.NameOrdering) for (name <- defaultMethodNames) { val methods = clazz.info.member(name).alternatives.map(_.symbol) diff --git a/compiler/src/dotty/tools/dotc/util/UniqList.scala b/compiler/src/dotty/tools/dotc/util/UniqList.scala deleted file mode 100644 index 1c7a45821b36..000000000000 --- a/compiler/src/dotty/tools/dotc/util/UniqList.scala +++ /dev/null @@ -1,21 +0,0 @@ -package dotty.tools.dotc.util - -opaque type UniqList[+A] >: Null <: AnyRef = List[A] - -object UniqList: - def empty: UniqList[Nothing] = Nil - extension [A](xs: UniqList[A]) - def foreach(f: A => Unit): Unit = xs.foreach(f) - def filter(p: A => Boolean): UniqList[A] = xs.filter(p) - def + (x: A): UniqList[A] = if xs.contains(x) then xs else xs :+ x - def & (ys: UniqList[A]): UniqList[A] = xs.filter(ys.contains) - def | (ys: UniqList[A]): UniqList[A] = (xs ::: ys).distinct - def toList: List[A] = xs - - class Builder[A] extends scala.collection.mutable.Builder[A, UniqList[A]]: - private val xs = new scala.collection.mutable.ListBuffer[A] - override def addOne(x: A): this.type = { xs += x; this } - override def addAll(ys: IterableOnce[A]): this.type = { xs ++= ys; this } - override def clear(): Unit = xs.clear() - override def result(): UniqList[A] = xs.distinct.toList - override def sizeHint(size: Int) = xs.sizeHint(size)