Skip to content

Commit

Permalink
Fix #401: Avoid asSeenFrom for types/bounds that don't depend on the …
Browse files Browse the repository at this point in the history
…prefix.

This mostly acts as a fast path. However, for the particular case
of static recursive match types, it more critically cuts some
infinite recursion.

Normally, when a match type refers to itself, it would do so
through a `ThisType`. Computing `asSeenFrom` for a `ThisType`
prefix already cuts the infinite recursion because it is, by
construction, a no-op.

However, for static recursive match types, the compiler makes them
refer to themselves through their public, static prefix. This
caused the infinite recursion observed in #401.

To avoid the issue, we use another shortcut to `asSeenFrom`: if
the type we map over does not contain any `ThisType`, we also
know it will be a no-op. We store that as a lazy val
`isPrefixDependent` of `TermSymbol` and `TypeMemberSymbol`, so
that they can take a short cut in `{type,bounds}AsSeenFrom`.
  • Loading branch information
sjrd committed Nov 29, 2023
1 parent b9dda99 commit 060847e
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 2 deletions.
2 changes: 2 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ lazy val tastyQuery =
mimaBinaryIssueFilters ++= {
import com.typesafe.tools.mima.core.*
Seq(
// private, not an issue
ProblemFilters.exclude[MissingClassProblem]("tastyquery.TypeOps$TypeFold"),
// Everything in tastyquery.reader is private[tastyquery] at most
ProblemFilters.exclude[Problem]("tastyquery.reader.*"),
)
Expand Down
10 changes: 8 additions & 2 deletions tasty-query/shared/src/main/scala/tastyquery/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,8 @@ object Symbols {
if local != null then local
else throw new IllegalStateException(s"$this was not assigned a declared type")

private lazy val isPrefixDependent: Boolean = TypeOps.isPrefixDependent(declaredType)

private[tastyquery] final def setParamSymss(paramSymss: List[ParamSymbolsClause]): this.type =
if myParamSymss != null then throw IllegalStateException(s"reassignment of paramSymss to $this")
myParamSymss = paramSymss
Expand Down Expand Up @@ -607,7 +609,8 @@ object Symbols {
TermRef(owner.staticOwnerPrefix, this)

final def typeAsSeenFrom(prefix: Prefix)(using Context): TypeOrMethodic =
declaredType.asSeenFrom(prefix, owner)
if isPrefixDependent then declaredType.asSeenFrom(prefix, owner)
else declaredType // fast path

private def isConstructor: Boolean =
owner.isClass && isMethod && name == nme.Constructor
Expand Down Expand Up @@ -731,9 +734,12 @@ object Symbols {

def declaredBounds: TypeBounds

private lazy val isPrefixDependent = TypeOps.isPrefixDependent(declaredBounds)

final def boundsAsSeenFrom(prefix: Prefix)(using Context): TypeBounds =
def default: TypeBounds =
declaredBounds.mapBounds(_.asSeenFrom(prefix, owner))
if isPrefixDependent then declaredBounds.mapBounds(_.asSeenFrom(prefix, owner))
else declaredBounds // fast path, but also important to cut infinite recursions

this match
case sym: ClassTypeParamSymbol =>
Expand Down
72 changes: 72 additions & 0 deletions tasty-query/shared/src/main/scala/tastyquery/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ import tastyquery.Types.*
import tastyquery.TypeMaps.*

private[tastyquery] object TypeOps:
/** Is the given type dependent at all on the prefix it will be seen from?
*
* In other words, does it contain any `ThisType`?
*
* If not, there is no need to map it through `asSeenFrom`.
*/
def isPrefixDependent(tp: TypeMappable): Boolean =
existsPart(tp)(_.isInstanceOf[ThisType])

def asSeenFrom(tp: TypeOrMethodic, pre: Prefix, cls: Symbol)(using Context): tp.ThisTypeMappableType =
pre match
case NoPrefix | _: PackageRef => tp
Expand Down Expand Up @@ -83,6 +92,69 @@ private[tastyquery] object TypeOps:
}
}

// Folds over types

def existsPart(tp: TypeMappable)(p: TypeMappable => Boolean): Boolean =
new TypeFold[Boolean] {
override def apply(z: Boolean, tp: TypeMappable): Boolean =
z || p(tp) || super.apply(z, tp)
}.apply(false, tp)
end existsPart

private class TypeFold[A] extends ((A, TypeMappable) => A):
def apply(z: A, tp: TypeMappable): A = tp match
case NoPrefix | _: PackageRef =>
z

case _: NothingType | _: AnyKindType | _: ThisType | _: ConstantType | _: ParamRef | _: RecThis =>
z
case tp: NamedType =>
apply(z, tp.prefix)
case tp: SuperType =>
apply(z, tp.thistpe)
case tp: AppliedType =>
tp.args.foldLeft(apply(z, tp.tycon))(this)
case tp: ByNameType =>
apply(z, tp.resultType)
case tp: RepeatedType =>
apply(z, tp.elemType)
case tp: LambdaType =>
apply(tp.paramInfos.foldLeft(z)(this), tp.resultType)
case tp: AnnotatedType =>
apply(z, tp.typ)
case tp: TypeRefinement =>
apply(apply(z, tp.parent), tp.refinedBounds)
case tp: TermRefinement =>
apply(apply(z, tp.parent), tp.refinedType)
case tp: RecType =>
apply(z, tp.parent)
case tp: MatchType =>
val z1 = apply(apply(z, tp.bound), tp.scrutinee)
tp.cases.foldLeft(z1)(apply(_, _))
case tp: SkolemType =>
apply(z, tp.tpe)
case tp: OrType =>
apply(apply(z, tp.first), tp.second)
case tp: AndType =>
apply(apply(z, tp.first), tp.second)

case tp: WildcardTypeArg =>
apply(z, tp.bounds)

case AbstractTypeBounds(low, high) =>
apply(apply(z, low), high)
case TypeAlias(alias) =>
apply(z, alias)

case tp: CustomTransientGroundType =>
throw IllegalArgumentException(s"Unexpected custom transient type: $tp")
end apply

def apply(z: A, caze: MatchTypeCase): A =
val z1 = caze.paramTypeBounds.foldLeft(z)(this)
apply(apply(z1, caze.pattern), caze.result)
end TypeFold

// Tests around `matches`

/** The implementation for `tp1.matches(tp2)`. */
Expand Down
12 changes: 12 additions & 0 deletions tasty-query/shared/src/test/scala/tastyquery/TypeSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3635,4 +3635,16 @@ class TypeSuite extends UnrestrictedUnpicklingSuite {
assert(clue(c.declaredType).isApplied(_.isRef(JCollectionClass), List(_.isBounded(_.isNothing, _.isRef(e)))))
}
}

testWithContext("recursive-match-types-bounds-as-seen-from-issue-401") {
val RecursiveMatchTypeSym =
ctx.findStaticType("simple_trees.RecursiveMatchType$package.RecursiveMatchType").asInstanceOf[TypeMemberSymbol]
val staticRef = RecursiveMatchTypeSym.staticRef

/* Here, we only make sure that calls below terminate.
* More elaborate resolution is tested through `WholeClasspathSuite` over `RecursiveMatchTypeTest`.
*/
RecursiveMatchTypeSym.boundsAsSeenFrom(staticRef.prefix)
staticRef.underlying
}
}
18 changes: 18 additions & 0 deletions test-sources/src/main/scala/simple_trees/RecursiveMatchType.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package simple_trees

type RecursiveMatchType[A <: Tuple] <: Tuple = A match
case hd *: tl => hd *: RecursiveMatchType[tl]
case EmptyTuple => EmptyTuple

object RecursiveMatchType:
inline def rec[A <: Tuple](a: A): RecursiveMatchType[A] =
inline a match
case b: (hd *: tl) => b.head *: rec(b.tail)
case _: EmptyTuple => EmptyTuple
end RecursiveMatchType

// must be in a separate TASTy file to trigger issue #401
object RecursiveMatchTypeTest:
inline def rec[A <: Tuple](x: A): RecursiveMatchType[A] =
RecursiveMatchType.rec(x)
end RecursiveMatchTypeTest

0 comments on commit 060847e

Please sign in to comment.