From 4c6e80f79850fd6e9be6729e39bb85158562c023 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 7 Aug 2024 11:20:08 +0200 Subject: [PATCH 1/3] Introduce provisional state for better cache reuse --- .../src/dotty/tools/dotc/core/Types.scala | 178 +++++++++++++++--- tests/pos/i20217.scala | 42 +++++ 2 files changed, 189 insertions(+), 31 deletions(-) create mode 100644 tests/pos/i20217.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 8181b3c83acf..814eac90240c 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -118,39 +118,126 @@ object Types extends TypeUtils { * a call to `resetInst`. This means all caches that rely on `isProvisional` * can likely end up returning stale results. */ - def isProvisional(using Context): Boolean = mightBeProvisional && testProvisional + // def isProvisional(using Context): Boolean = mightBeProvisional && testProvisional + def isProvisional(using Context): Boolean = mightBeProvisional && !currentProvisionalState.isEmpty - private def testProvisional(using Context): Boolean = + type ProvisionalState = util.HashMap[Type, Type] + + def currentProvisionalState(using Context): ProvisionalState = + val state: ProvisionalState = util.HashMap() + // Compared to `testProvisional`, we don't use short-circuiting or, + // because we want to collect all provisional types. class ProAcc extends TypeAccumulator[Boolean]: - override def apply(x: Boolean, t: Type) = x || test(t, this) + override def apply(x: Boolean, t: Type) = x | test(t, this) def test(t: Type, theAcc: TypeAccumulator[Boolean] | Null): Boolean = if t.mightBeProvisional then t.mightBeProvisional = t match case t: TypeRef => - t.currentSymbol.isProvisional || !t.currentSymbol.isStatic && { + if t.currentSymbol.isProvisional then + // When t is a TypeRef and its symbol is provisional, + // t will be considered provisional and its state is always updating. + // We store itself as info. + state(t) = t + true + else if !t.currentSymbol.isStatic then (t: Type).mightBeProvisional = false // break cycles - test(t.prefix, theAcc) - || t.denot.infoOrCompleter.match - case info: LazyType => true - case info: AliasingBounds => test(info.alias, theAcc) - case TypeBounds(lo, hi) => test(lo, theAcc) || test(hi, theAcc) - case _ => false - } + if test(t.prefix, theAcc) then + // If the prefix is provisional, some provisional type from it + // must have been added to state, so we don't need to add t. + true + else t.denot.infoOrCompleter.match + case info: LazyType => + state(t) = info + true + case info: AliasingBounds => + test(info.alias, theAcc) + case TypeBounds(lo, hi) => + test(lo, theAcc) | test(hi, theAcc) + case _ => + // If a TypeRef has been fully completed, it is no longer provisional, + // so we don't need to traverse its info. + false + else false case t: TermRef => !t.currentSymbol.isStatic && test(t.prefix, theAcc) case t: AppliedType => - t.fold(false, (x, tp) => x || test(tp, theAcc)) + t.fold(false, (x, tp) => x | test(tp, theAcc)) case t: TypeVar => - !t.isPermanentlyInstantiated || test(t.permanentInst, theAcc) + if t.isPermanentlyInstantiated then + test(t.permanentInst, theAcc) + else + val inst = t.instanceOpt + if inst.exists then + // We want to store the temporary instance to the state + // in order to reuse the cache when possible. + state(t) = inst + test(inst, theAcc) + else + // When t is a TypeVar and does not have an instancication, + // we store itself as info. + state(t) = t + true case t: LazyRef => - !t.completed || test(t.ref, theAcc) + if !t.completed then + // When t is a LazyRef and is not completed, + // we store itself as info. + state(t) = t + true + else + test(t.ref, theAcc) case _ => (if theAcc != null then theAcc else ProAcc()).foldOver(false, t) end if t.mightBeProvisional end test test(this, null) - end testProvisional + state + end currentProvisionalState + + def isStateUpToDate( + currentState: ProvisionalState, + lastState: ProvisionalState | Null) + (using Context): Boolean = + lastState != null + && currentState.size == lastState.size + && currentState.iterator.forall: (tp, info) => + lastState.contains(tp) && { + tp match + case tp: TypeRef => (info ne tp) && (info eq lastState(tp)) + case _=> info eq lastState(tp) + } + + // private def testProvisional(using Context): Boolean = + // class ProAcc extends TypeAccumulator[Boolean]: + // override def apply(x: Boolean, t: Type) = x || test(t, this) + // def test(t: Type, theAcc: TypeAccumulator[Boolean] | Null): Boolean = + // if t.mightBeProvisional then + // t.mightBeProvisional = t match + // case t: TypeRef => + // t.currentSymbol.isProvisional || !t.currentSymbol.isStatic && { + // (t: Type).mightBeProvisional = false // break cycles + // test(t.prefix, theAcc) + // || t.denot.infoOrCompleter.match + // case info: LazyType => true + // case info: AliasingBounds => test(info.alias, theAcc) + // case TypeBounds(lo, hi) => test(lo, theAcc) || test(hi, theAcc) + // case _ => false + // } + // case t: TermRef => + // !t.currentSymbol.isStatic && test(t.prefix, theAcc) + // case t: AppliedType => + // t.fold(false, (x, tp) => x || test(tp, theAcc)) + // case t: TypeVar => + // !t.isPermanentlyInstantiated || test(t.permanentInst, theAcc) + // case t: LazyRef => + // !t.completed || test(t.ref, theAcc) + // case _ => + // (if theAcc != null then theAcc else ProAcc()).foldOver(false, t) + // end if + // t.mightBeProvisional + // end test + // test(this, null) + // end testProvisional /** Is this type different from NoType? */ final def exists: Boolean = this.ne(NoType) @@ -1311,7 +1398,10 @@ object Types extends TypeUtils { final def widen(using Context): Type = this match case _: TypeRef | _: MethodOrPoly => this // fast path for most frequent cases case tp: TermRef => // fast path for next most frequent case - if tp.isOverloaded then tp else tp.underlying.widen + // Don't call `isOverloaded` and `underlying` on `tp` directly, + // to avoid computing provisional state twice. + val denot = tp.denot + if denot.isOverloaded then tp else denot.info.widen case tp: SingletonType => tp.underlying.widen case tp: ExprType => tp.resultType.widen case tp => @@ -2305,10 +2395,12 @@ object Types extends TypeUtils { private var myName: Name | Null = null private var lastDenotation: Denotation | Null = null + private var lastDenotationProvState: ProvisionalState | Null = null private var lastSymbol: Symbol | Null = null private var checkedPeriod: Period = Nowhere private var myStableHash: Byte = 0 private var mySignature: Signature = uninitialized + private var mySignatureProvState: ProvisionalState | Null = null private var mySignatureRunId: Int = NoRunId // Invariants: @@ -2343,9 +2435,12 @@ object Types extends TypeUtils { else if ctx.erasedTypes then atPhase(erasurePhase)(computeSignature) else symbol.asSeenFrom(prefix).signature - if ctx.runId != mySignatureRunId then + val currentState = currentProvisionalState + if ctx.runId != mySignatureRunId + || !isStateUpToDate(currentState, mySignatureProvState) then mySignature = computeSignature - if !mySignature.isUnderDefined && !isProvisional then mySignatureRunId = ctx.runId + mySignatureProvState = currentState + if !mySignature.isUnderDefined then mySignatureRunId = ctx.runId mySignature end signature @@ -2356,7 +2451,9 @@ object Types extends TypeUtils { * some symbols change their signature at erasure. */ private def currentSignature(using Context): Signature = - if ctx.runId == mySignatureRunId then mySignature + if ctx.runId == mySignatureRunId + && isStateUpToDate(currentProvisionalState, mySignatureProvState) + then mySignature else val lastd = lastDenotation if lastd != null then sigFromDenot(lastd) @@ -2434,7 +2531,10 @@ object Types extends TypeUtils { val lastd = lastDenotation.asInstanceOf[Denotation] // Even if checkedPeriod == now we still need to recheck lastDenotation.validFor // as it may have been mutated by SymDenotation#installAfter - if checkedPeriod.code != NowhereCode && lastd.validFor.contains(ctx.period) then lastd + if checkedPeriod.code != NowhereCode + && isStateUpToDate(prefix.currentProvisionalState, lastDenotationProvState) + && lastd.validFor.contains(ctx.period) + then lastd else computeDenot private def computeDenot(using Context): Denotation = { @@ -2468,14 +2568,18 @@ object Types extends TypeUtils { finish(symd.current) } + def isLastDenotValid = + checkedPeriod.code != NowhereCode + && isStateUpToDate(prefix.currentProvisionalState, lastDenotationProvState) + lastDenotation match { case lastd0: SingleDenotation => val lastd = lastd0.skipRemoved - if lastd.validFor.runId == ctx.runId && checkedPeriod.code != NowhereCode then + if lastd.validFor.runId == ctx.runId && isLastDenotValid then finish(lastd.current) else lastd match { case lastd: SymDenotation => - if stillValid(lastd) && checkedPeriod.code != NowhereCode then finish(lastd.current) + if stillValid(lastd) && isLastDenotValid then finish(lastd.current) else finish(memberDenot(lastd.initial.name, allowPrivate = false)) case _ => fromDesignator @@ -2566,7 +2670,8 @@ object Types extends TypeUtils { lastDenotation = denot lastSymbol = denot.symbol - checkedPeriod = if (prefix.isProvisional) Nowhere else ctx.period + lastDenotationProvState = prefix.currentProvisionalState + checkedPeriod = ctx.period designator match { case sym: Symbol if designator ne lastSymbol.nn => designator = lastSymbol.asInstanceOf[Designator{ type ThisName = self.ThisName }] @@ -3849,14 +3954,18 @@ object Types extends TypeUtils { sealed abstract class MethodOrPoly extends UncachedGroundType with LambdaType with MethodicType { // Invariants: - // (1) mySignatureRunId != NoRunId => mySignature != null - // (2) myJavaSignatureRunId != NoRunId => myJavaSignature != null + // (1) mySignatureRunId != NoRunId => mySignature != null + // (2) myJavaSignatureRunId != NoRunId => myJavaSignature != null + // (3) myScala2SignatureRunId != NoRunId => myScala2Signature != null private var mySignature: Signature = uninitialized + private var mySignatureProvState: ProvisionalState | Null = null private var mySignatureRunId: Int = NoRunId private var myJavaSignature: Signature = uninitialized + private var myJavaSignatureProvState: ProvisionalState | Null = null private var myJavaSignatureRunId: Int = NoRunId private var myScala2Signature: Signature = uninitialized + private var myScala2SignatureProvState: ProvisionalState | Null = null private var myScala2SignatureRunId: Int = NoRunId /** If `isJava` is false, the Scala signature of this method. Otherwise, its Java signature. @@ -3892,21 +4001,28 @@ object Types extends TypeUtils { case tp: PolyType => resultSignature.prependTypeParams(tp.paramNames.length) + val currentState = currentProvisionalState sourceLanguage match case SourceLanguage.Java => - if ctx.runId != myJavaSignatureRunId then + if ctx.runId != myJavaSignatureRunId + || !isStateUpToDate(currentState, myJavaSignatureProvState) then myJavaSignature = computeSignature - if !myJavaSignature.isUnderDefined && !isProvisional then myJavaSignatureRunId = ctx.runId + myJavaSignatureProvState = currentState + if !myJavaSignature.isUnderDefined then myJavaSignatureRunId = ctx.runId myJavaSignature case SourceLanguage.Scala2 => - if ctx.runId != myScala2SignatureRunId then + if ctx.runId != myScala2SignatureRunId + || !isStateUpToDate(currentState, myScala2SignatureProvState) then myScala2Signature = computeSignature - if !myScala2Signature.isUnderDefined && !isProvisional then myScala2SignatureRunId = ctx.runId + myScala2SignatureProvState = currentState + if !myScala2Signature.isUnderDefined then myScala2SignatureRunId = ctx.runId myScala2Signature case SourceLanguage.Scala3 => - if ctx.runId != mySignatureRunId then + if ctx.runId != mySignatureRunId + || !isStateUpToDate(currentState, mySignatureProvState) then mySignature = computeSignature - if !mySignature.isUnderDefined && !isProvisional then mySignatureRunId = ctx.runId + mySignatureProvState = currentState + if !mySignature.isUnderDefined then mySignatureRunId = ctx.runId mySignature end signature diff --git a/tests/pos/i20217.scala b/tests/pos/i20217.scala new file mode 100644 index 000000000000..0f4e0a4c9620 --- /dev/null +++ b/tests/pos/i20217.scala @@ -0,0 +1,42 @@ +// A simplifies version of PartiallyAppliedStruct from smithy4s + +trait Field[S, A] +trait Schema[S] + +class PartiallyAppliedStruct[S](): + def apply[A0](a0: Field[S, A0])(const: (A0) => S): Schema[S] = ??? + def apply[A0, A1](a0: Field[S, A0], a1: Field[S, A1])(const: (A0, A1) => S): Schema[S] = ??? + def apply[A0, A1, A2](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2])(const: (A0, A1, A2) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3])(const: (A0, A1, A2, A3) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4])(const: (A0, A1, A2, A3, A4) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5])(const: (A0, A1, A2, A3, A4, A5) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6])(const: (A0, A1, A2, A3, A4, A5, A6) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7])(const: (A0, A1, A2, A3, A4, A5, A6, A7) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16], a17: Field[S, A17])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16], a17: Field[S, A17], a18: Field[S, A18])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16], a17: Field[S, A17], a18: Field[S, A18], a19: Field[S, A19])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16], a17: Field[S, A17], a18: Field[S, A18], a19: Field[S, A19], a20: Field[S, A20])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20) => S): Schema[S] = ??? + def apply[A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20, A21](a0: Field[S, A0], a1: Field[S, A1], a2: Field[S, A2], a3: Field[S, A3], a4: Field[S, A4], a5: Field[S, A5], a6: Field[S, A6], a7: Field[S, A7], a8: Field[S, A8], a9: Field[S, A9], a10: Field[S, A10], a11: Field[S, A11], a12: Field[S, A12], a13: Field[S, A13], a14: Field[S, A14], a15: Field[S, A15], a16: Field[S, A16], a17: Field[S, A17], a18: Field[S, A18], a19: Field[S, A19], a20: Field[S, A20], a21: Field[S, A21])(const: (A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20, A21) => S): Schema[S] = ??? + +def struct[S] = new PartiallyAppliedStruct[S]() + + +def test[S, T](a: Field[S, Int], b: Field[S, String], c: Field[S, T], defaultValue: S) = + val s1: Schema[S] = struct(a)(_ => defaultValue) + val s2: Schema[S] = struct(a, b)((_, _) => defaultValue) + val s3: Schema[S] = struct(a, b, c)((_, _, _) => defaultValue) + val s4: Schema[S] = struct(a, b, c, a, b, c)((_, _, _, _, _, _) => defaultValue) + + val s5 = struct(a)(_ => defaultValue) + val s6 = struct(a, b)((_, _) => defaultValue) + val s7 = struct(a, b, c)((_, _, _) => defaultValue) + val s8 = struct(a, b, c, a, b, c)((_, _, _, _, _, _) => defaultValue) From 1df045c2f3cdbbfec537cb44211a92601e4f0462 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 4 Sep 2024 15:13:13 +0200 Subject: [PATCH 2/3] Resolve review commts --- .../src/dotty/tools/dotc/core/Types.scala | 106 +++++++----------- 1 file changed, 41 insertions(+), 65 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 814eac90240c..4869375d8c94 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -118,14 +118,20 @@ object Types extends TypeUtils { * a call to `resetInst`. This means all caches that rely on `isProvisional` * can likely end up returning stale results. */ - // def isProvisional(using Context): Boolean = mightBeProvisional && testProvisional - def isProvisional(using Context): Boolean = mightBeProvisional && !currentProvisionalState.isEmpty + def isProvisional(using Context): Boolean = mightBeProvisional && currentProvisionalState != null - type ProvisionalState = util.HashMap[Type, Type] + // The provisonal state of a type stores the parts which might be changed and their + // info at a given point. + // For example, a `TypeVar` is provisional until it is permently instantiated, + // and its info is the current instantiation. + type ProvisionalState = util.HashMap[Type, Type] | Null def currentProvisionalState(using Context): ProvisionalState = - val state: ProvisionalState = util.HashMap() - // Compared to `testProvisional`, we don't use short-circuiting or, + var state: ProvisionalState = null + inline def record(tp: Type, info: Type): Unit = + if state == null then state = util.HashMap() + state.uncheckedNN(tp) = info + // Compared to previous `testProvisional`, we don't use short-circuiting or (`||`), // because we want to collect all provisional types. class ProAcc extends TypeAccumulator[Boolean]: override def apply(x: Boolean, t: Type) = x | test(t, this) @@ -137,7 +143,7 @@ object Types extends TypeUtils { // When t is a TypeRef and its symbol is provisional, // t will be considered provisional and its state is always updating. // We store itself as info. - state(t) = t + record(t, t) true else if !t.currentSymbol.isStatic then (t: Type).mightBeProvisional = false // break cycles @@ -147,7 +153,7 @@ object Types extends TypeUtils { true else t.denot.infoOrCompleter.match case info: LazyType => - state(t) = info + record(t, info) true case info: AliasingBounds => test(info.alias, theAcc) @@ -170,18 +176,18 @@ object Types extends TypeUtils { if inst.exists then // We want to store the temporary instance to the state // in order to reuse the cache when possible. - state(t) = inst + record(t, inst) test(inst, theAcc) else - // When t is a TypeVar and does not have an instancication, + // When t is a TypeVar and does not have an instantiation, // we store itself as info. - state(t) = t + record(t, t) true case t: LazyRef => if !t.completed then // When t is a LazyRef and is not completed, // we store itself as info. - state(t) = t + record(t, t) true else test(t.ref, theAcc) @@ -196,48 +202,17 @@ object Types extends TypeUtils { def isStateUpToDate( currentState: ProvisionalState, - lastState: ProvisionalState | Null) + lastState: ProvisionalState) (using Context): Boolean = - lastState != null - && currentState.size == lastState.size - && currentState.iterator.forall: (tp, info) => - lastState.contains(tp) && { - tp match - case tp: TypeRef => (info ne tp) && (info eq lastState(tp)) - case _=> info eq lastState(tp) - } - - // private def testProvisional(using Context): Boolean = - // class ProAcc extends TypeAccumulator[Boolean]: - // override def apply(x: Boolean, t: Type) = x || test(t, this) - // def test(t: Type, theAcc: TypeAccumulator[Boolean] | Null): Boolean = - // if t.mightBeProvisional then - // t.mightBeProvisional = t match - // case t: TypeRef => - // t.currentSymbol.isProvisional || !t.currentSymbol.isStatic && { - // (t: Type).mightBeProvisional = false // break cycles - // test(t.prefix, theAcc) - // || t.denot.infoOrCompleter.match - // case info: LazyType => true - // case info: AliasingBounds => test(info.alias, theAcc) - // case TypeBounds(lo, hi) => test(lo, theAcc) || test(hi, theAcc) - // case _ => false - // } - // case t: TermRef => - // !t.currentSymbol.isStatic && test(t.prefix, theAcc) - // case t: AppliedType => - // t.fold(false, (x, tp) => x || test(tp, theAcc)) - // case t: TypeVar => - // !t.isPermanentlyInstantiated || test(t.permanentInst, theAcc) - // case t: LazyRef => - // !t.completed || test(t.ref, theAcc) - // case _ => - // (if theAcc != null then theAcc else ProAcc()).foldOver(false, t) - // end if - // t.mightBeProvisional - // end test - // test(this, null) - // end testProvisional + (currentState eq lastState) + || currentState != null && lastState != null + && currentState.size == lastState.size + && currentState.iterator.forall: (tp, info) => + lastState.contains(tp) && { + tp match + case tp: TypeRef => (info ne tp) && (info eq lastState(tp)) + case _ => info eq lastState(tp) + } /** Is this type different from NoType? */ final def exists: Boolean = this.ne(NoType) @@ -1398,8 +1373,6 @@ object Types extends TypeUtils { final def widen(using Context): Type = this match case _: TypeRef | _: MethodOrPoly => this // fast path for most frequent cases case tp: TermRef => // fast path for next most frequent case - // Don't call `isOverloaded` and `underlying` on `tp` directly, - // to avoid computing provisional state twice. val denot = tp.denot if denot.isOverloaded then tp else denot.info.widen case tp: SingletonType => tp.underlying.widen @@ -1414,10 +1387,12 @@ object Types extends TypeUtils { /** Widen from singleton type to its underlying non-singleton * base type by applying one or more `underlying` dereferences. */ - final def widenSingleton(using Context): Type = stripped match { - case tp: SingletonType if !tp.isOverloaded => tp.underlying.widenSingleton + final def widenSingleton(using Context): Type = stripped match + case tp: TermRef => + val denot = tp.denot + if denot.isOverloaded then this else denot.info.widenSingleton + case tp: SingletonType => tp.underlying.widenSingleton case _ => this - } /** Widen from TermRef to its underlying non-termref * base type, while also skipping Expr types. @@ -2395,12 +2370,12 @@ object Types extends TypeUtils { private var myName: Name | Null = null private var lastDenotation: Denotation | Null = null - private var lastDenotationProvState: ProvisionalState | Null = null + private var lastDenotationProvState: ProvisionalState = null private var lastSymbol: Symbol | Null = null private var checkedPeriod: Period = Nowhere private var myStableHash: Byte = 0 private var mySignature: Signature = uninitialized - private var mySignatureProvState: ProvisionalState | Null = null + private var mySignatureProvState: ProvisionalState = null private var mySignatureRunId: Int = NoRunId // Invariants: @@ -2473,7 +2448,9 @@ object Types extends TypeUtils { final def symbol(using Context): Symbol = // We can rely on checkedPeriod (unlike in the definition of `denot` below) // because SymDenotation#installAfter never changes the symbol - if (checkedPeriod.code == ctx.period.code) lastSymbol.asInstanceOf[Symbol] + if checkedPeriod.code == ctx.period.code + && isStateUpToDate(prefix.currentProvisionalState, lastDenotationProvState) then + lastSymbol.asInstanceOf[Symbol] else computeSymbol private def computeSymbol(using Context): Symbol = @@ -2482,7 +2459,6 @@ object Types extends TypeUtils { if (sym.isValidInCurrentRun) sym else denot.symbol case name => (if (denotationIsCurrent) lastDenotation.asInstanceOf[Denotation] else denot).symbol - if checkedPeriod.code != NowhereCode then checkedPeriod = ctx.period result /** There is a denotation computed which is valid (somewhere in) the @@ -2532,8 +2508,8 @@ object Types extends TypeUtils { // Even if checkedPeriod == now we still need to recheck lastDenotation.validFor // as it may have been mutated by SymDenotation#installAfter if checkedPeriod.code != NowhereCode - && isStateUpToDate(prefix.currentProvisionalState, lastDenotationProvState) && lastd.validFor.contains(ctx.period) + && isStateUpToDate(prefix.currentProvisionalState, lastDenotationProvState) then lastd else computeDenot @@ -3959,13 +3935,13 @@ object Types extends TypeUtils { // (3) myScala2SignatureRunId != NoRunId => myScala2Signature != null private var mySignature: Signature = uninitialized - private var mySignatureProvState: ProvisionalState | Null = null + private var mySignatureProvState: ProvisionalState = null private var mySignatureRunId: Int = NoRunId private var myJavaSignature: Signature = uninitialized - private var myJavaSignatureProvState: ProvisionalState | Null = null + private var myJavaSignatureProvState: ProvisionalState = null private var myJavaSignatureRunId: Int = NoRunId private var myScala2Signature: Signature = uninitialized - private var myScala2SignatureProvState: ProvisionalState | Null = null + private var myScala2SignatureProvState: ProvisionalState = null private var myScala2SignatureRunId: Int = NoRunId /** If `isJava` is false, the Scala signature of this method. Otherwise, its Java signature. From 61d0aad0b6ab5fec05d10b83d0f37a581ec0d6af Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 4 Sep 2024 18:19:17 +0200 Subject: [PATCH 3/3] Add back the checkedPeriod update --- compiler/src/dotty/tools/dotc/core/Types.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 4869375d8c94..4fd879eb05d8 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2459,6 +2459,7 @@ object Types extends TypeUtils { if (sym.isValidInCurrentRun) sym else denot.symbol case name => (if (denotationIsCurrent) lastDenotation.asInstanceOf[Denotation] else denot).symbol + if checkedPeriod.code != NowhereCode then checkedPeriod = ctx.period result /** There is a denotation computed which is valid (somewhere in) the