Skip to content

Commit b872039

Browse files
authored
Quotes reflect: sort the typeMembers output list and filter out non-members (#22876)
The typeMember currently returns a List (implying some kind of ordering), even though the order of the elements can be unpredictable. This is because the underlying methods operate on Sets, before wrapping those into a Seq. I believe the simplest solution is to just sort the obtained List as part of the quotes reflect implementation: first depending on the location (which classlike contains it), then depending on if it is a type parameter (and the declaration order of it), then for non-type params lexicographically (ideally it would also be sorted by declaration order, but I can't really find the use case for that and it would easily be the costliest part). The reasons for choosing to sort are: * rewriting the memberDenot to remember the ordering of items (like suggested in the linked issue thread) is dangerous. LinkedHashMap the only hashmap which keeps the insertion order, but it currently has no immutable version (so a simple mistake could spiral across the whole compiler, esp. easy to happen as the collected names are cached there) and it would add an ever so slight performance cost to a critical part of the compiler. * This all might suggest that a quotes reflect specific rewrite would be better (since we end up redoing some of the calculations done as part of the `lookupPrefix.typeMembers`), but the costliest part, filtering through all of the declarations from all of the parents, we do not have to touch as part of this sort. We can also reuse the caches (while making sure we do not miss any type members which were included in the previous implementation), since we just sort that part lexicographically. We also now filter out the non-existent Symbols, which can appear like in #22483. The whole reason why that happens is because after collecting the member names in `memberDenots` we wrap them in `member` to get their Denotation/Symbol. If that returns a NoSymbol, it's because we cannot access them as part of the symbol (so any kind of selection there will not work either). Because of this, I am comfortable with just filtering them out. Fixes #22472 Fixes #22483
2 parents ad95e58 + 36f7810 commit b872039

File tree

6 files changed

+98
-2
lines changed

6 files changed

+98
-2
lines changed

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,23 @@ object Types extends TypeUtils {
10511051
buf.toList
10521052
}
10531053

1054+
/** For use in quotes reflect.
1055+
* A bit slower than the usual approach due to the use of LinkedHashSet.
1056+
**/
1057+
def sortedParents(using Context): mutable.LinkedHashSet[Type] = this match
1058+
case tp: ClassInfo =>
1059+
mutable.LinkedHashSet(tp) | mutable.LinkedHashSet(tp.declaredParents.flatMap(_.sortedParents.toList)*)
1060+
case tp: RefinedType =>
1061+
tp.parent.sortedParents
1062+
case tp: TypeProxy =>
1063+
tp.superType.sortedParents
1064+
case tp: AndType =>
1065+
tp.tp1.sortedParents | tp.tp2.sortedParents
1066+
case tp: OrType =>
1067+
tp.tp1.sortedParents & tp.tp2.sortedParents
1068+
case _ =>
1069+
mutable.LinkedHashSet()
1070+
10541071
/** The set of abstract term members of this type. */
10551072
final def abstractTermMembers(using Context): Seq[SingleDenotation] = {
10561073
record("abstractTermMembers")

compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3018,7 +3018,33 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
30183018
def memberTypes: List[Symbol] =
30193019
self.typeRef.decls.filter(_.isType)
30203020
def typeMembers: List[Symbol] =
3021-
lookupPrefix.typeMembers.map(_.symbol).toList
3021+
// lookupPrefix.typeMembers currently returns a Set wrapped into a unsorted Seq,
3022+
// so we try to sort that here (see discussion: https://github.com/scala/scala3/issues/22472),
3023+
// without adding too much of a performance hit.
3024+
// It first sorts by parents, then for type params by their positioning, then for members
3025+
// derived from declarations it sorts them by their name lexicographically
3026+
val parentsMap = lookupPrefix.sortedParents.map(_.typeSymbol).zipWithIndex.toList.toMap
3027+
val unsortedTypeMembers = lookupPrefix.typeMembers.map(_.symbol).filter(_.exists).toList
3028+
unsortedTypeMembers.sortWith {
3029+
case (typeA, typeB) =>
3030+
val msg = "Unknown type member found. Please consider reporting the issue to the compiler. "
3031+
assert(parentsMap.contains(typeA.owner), msg)
3032+
assert(parentsMap.contains(typeB.owner), msg)
3033+
val parentPlacementA = parentsMap(typeA.owner)
3034+
val parentPlacementB = parentsMap(typeB.owner)
3035+
if (parentPlacementA == parentPlacementB) then
3036+
if typeA.isTypeParam && typeB.isTypeParam then
3037+
// put type params at the beginning (and sort them by declaration order)
3038+
val pl = typeA.owner
3039+
val typeParamPositionMap = pl.typeParams.map(_.asInstanceOf[Symbol]).zipWithIndex.toMap
3040+
typeParamPositionMap(typeA) < typeParamPositionMap(typeB)
3041+
else if typeA.isTypeParam then true
3042+
else if typeB.isTypeParam then false
3043+
else
3044+
// sort by name lexicographically
3045+
typeA.name.toString().compareTo(typeB.name.toString()) < 0
3046+
else parentPlacementA < parentPlacementB
3047+
}.map(_.asInstanceOf[Symbol])
30223048

30233049
def declarations: List[Symbol] =
30243050
self.typeRef.info.decls.toList

tests/run-macros/i14902.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
List(X)
2-
List(X, Y, Z)
2+
List(Y, Z, X)
33
List(X)
44
List(Y, Z)

tests/run-macros/type-members.check

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class FooSmall: List(type A, type B, type C, type D)
2+
class FooLarge: List(type A, type B, type C, type D, type E)
3+
type FooUnion: List()
4+
type FooAnd: List(type A, type B, type C, type D, type E)
5+
trait CLS1: List(type A, type B, type C, type B1, type B2, type A3, type B3, type B4)
6+
type SharedAnd1: List(type B, type Shared, type A, type C)
7+
type SharedAnd2: List(type C, type Shared, type A, type B)
8+
type SharedUnion: List(type A, type Shared)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package example
2+
3+
import scala.quoted.*
4+
5+
object Macro {
6+
inline def typeMembers[T <: AnyKind]: String = ${ typeMembersImpl[T] }
7+
8+
def typeMembersImpl[T <: AnyKind: Type](using quotes: Quotes): Expr[String] = {
9+
import quotes.reflect.*
10+
Expr(s"${TypeRepr.of[T].typeSymbol}: ${TypeRepr.of[T].typeSymbol.typeMembers.toString}")
11+
}
12+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import example.Macro
2+
3+
class FooSmall[A, B] { type D; type C }
4+
class FooLarge[A, B, C] { type E; type D }
5+
6+
type FooUnion[A, B] = FooSmall[A, B] | FooLarge[A, B, Int]
7+
type FooAnd[A, B] = FooSmall[A, B] & FooLarge[A, B, Int]
8+
9+
trait CLS4[A] { type B4 }
10+
trait CLS3[A] extends CLS4[A] { type B3; type A3 }
11+
trait CLS2[A] { type B2 }
12+
trait CLS1[A, B, C] extends CLS2[A] with CLS3[B] { type B1 }
13+
14+
trait SharedParent[A] { type Shared }
15+
trait SharedA[A] extends SharedParent[A] { type B }
16+
trait SharedB[A] extends SharedParent[A] { type C }
17+
type SharedAnd1[A] = SharedA[A] & SharedB[A]
18+
type SharedAnd2[A] = SharedB[A] & SharedA[A]
19+
type SharedUnion[A] = SharedA[A] | SharedB[A]
20+
21+
@main def Test(): Unit = {
22+
println(Macro.typeMembers[FooSmall])
23+
println(Macro.typeMembers[FooLarge])
24+
25+
println(Macro.typeMembers[FooUnion])
26+
println(Macro.typeMembers[FooAnd])
27+
28+
println(Macro.typeMembers[CLS1])
29+
30+
println(Macro.typeMembers[SharedAnd1])
31+
println(Macro.typeMembers[SharedAnd2])
32+
println(Macro.typeMembers[SharedUnion])
33+
}

0 commit comments

Comments
 (0)