Skip to content

Commit

Permalink
only cache baseTypes when gadtState is empty
Browse files Browse the repository at this point in the history
  • Loading branch information
EugeneFlesselle committed Jan 29, 2024
1 parent 1716bcd commit bd25160
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 3 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ object Phases {
final def sameParentsStartId: Int = mySameParentsStartId
// id of first phase where all symbols are guaranteed to have the same parents as in this phase
final def sameBaseTypesStartId: Int = mySameBaseTypesStartId
// id of first phase where all symbols are guaranteed to have the same base tpyes as in this phase
// id of first phase where all symbols are guaranteed to have the same base types as in this phase

protected[Phases] def init(base: ContextBase, start: Int, end: Int): Unit = {
if (start >= FirstPhaseId)
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2192,7 +2192,8 @@ object SymDenotations {
Stats.record("basetype cache entries")
if (!baseTp.exists) Stats.record("basetype cache NoTypes")
}
if (!tp.isProvisional && !CapturingType.isUncachable(tp))
if !(tp.isProvisional || CapturingType.isUncachable(tp) || ctx.gadt.isNarrowing) then
// NOTE alternative: always try with empty gadt ctx first, can then always use cache, o.w. can try with ctx but no caching
btrCache(tp) = baseTp
else
btrCache.remove(tp) // Remove any potential sentinel value
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4075,7 +4075,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
case CompareResult.OKwithGADTUsed
if pt.isValueType
&& !inContext(ctx.fresh.setGadtState(GadtState(GadtConstraint.empty))) {
val res = (tree.tpe.widenExpr frozen_<:< pt)
// tree.tpe.classSymbol.denot.asClass.invalidateBaseTypeCache() // NOTE fixes the issue
val res = trace(i"unnecessary GADT for $tree: ${tree.tpe.widenExpr} vs $pt in ${ctx.source}"):
tree.tpe.widenExpr frozen_<:< pt
if res then
// we overshot; a cast is not needed, after all.
gadts.println(i"unnecessary GADTused for $tree: ${tree.tpe.widenExpr} vs $pt in ${ctx.source}")
Expand Down
13 changes: 13 additions & 0 deletions tests/pos/i19521.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

final abstract class PLet

sealed trait Expr[+P]
case class ELet[+A](name: String, expr: Expr[A]) extends Expr[A | PLet]

def go[P](e: Expr[P]): P = e match
case ELet(_, _) =>
val x: Expr[P] | ELet[P] = ???
val y: Expr[P] = x // conforms iff using gadt constraints
// error before changes: cast from gadt reasoning was not inserted because
// `Expr[P]` was erronously cached as a baseType of `Expr[P] | ELet[P]` (only true with gadt constraints)
???

0 comments on commit bd25160

Please sign in to comment.