From a4d1a8f60addaa8151bbcd4b36ee53f97adae193 Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Thu, 20 Dec 2018 13:00:00 +0300 Subject: [PATCH] Get rid of FUNCTOR slice, use lazy value in ContractDecription instead Functor is an imperative representation of function's contract (contrary to ContractDescription, which is a declarative one). ContractDescription is convenient when we deal with sources of contracts declarations (binaries, source), while Functors are convenient for analyzing code with contracts. It means that we have to convert ContractDescription into Functor when we start working with contracts. This computation isn't trivial, and Functor and ContractDescription are in 1-1 correspondence, so we would like to cache Functor for each ContractDescription somewhere. We used to do this in binding trace, in slice FUNCTOR. Now, it turns out that this approach causes "Rewrite at slice" exception, see KT-28847. We won't go into details of why that happens here, you can see the issue comments for details (but be prepared for the very long and nitty-gritty story) This commit removes the problematic slice and introduces another approach, where Functor is attached to the ContractDescription, computed lazily and cached here. ^KT-28847 Fixed --- .../kotlin/contracts/ContractDeserializerImpl.kt | 15 ++++++++++----- .../kotlin/contracts/EffectsExtractingVisitor.kt | 8 +++----- .../contracts/parsing/ContractParsingServices.kt | 5 +++-- .../parsing/PsiContractParserDispatcher.kt | 6 ++++-- .../jetbrains/kotlin/resolve/BindingContext.java | 1 - .../contracts/description/ContractDescription.kt | 16 +++++++++++++++- .../ContractInterpretationDispatcher.kt | 7 +------ .../contracts/dsl/rewriteAtSliceFunctor.txt | 8 ++++---- .../DeserializerForClassfileDecompiler.kt | 2 +- .../kotlinJavascriptPackageFragmentProvider.kt | 2 +- ...nanDeserializedPackageFragmentsFactoryImpl.kt | 2 +- 11 files changed, 43 insertions(+), 29 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/ContractDeserializerImpl.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/ContractDeserializerImpl.kt index 81d69692891..84b1b2a4956 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/ContractDeserializerImpl.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/ContractDeserializerImpl.kt @@ -28,10 +28,14 @@ import org.jetbrains.kotlin.metadata.deserialization.isInstanceType import org.jetbrains.kotlin.serialization.deserialization.ContractDeserializer import org.jetbrains.kotlin.serialization.deserialization.DeserializationConfiguration import org.jetbrains.kotlin.serialization.deserialization.TypeDeserializer +import org.jetbrains.kotlin.storage.StorageManager import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.utils.addIfNotNull -class ContractDeserializerImpl(private val configuration: DeserializationConfiguration) : ContractDeserializer { +class ContractDeserializerImpl( + private val configuration: DeserializationConfiguration, + private val storageManager: StorageManager +) : ContractDeserializer { override fun deserializeContractFromFunction( proto: ProtoBuf.Function, ownerFunction: FunctionDescriptor, @@ -42,7 +46,7 @@ class ContractDeserializerImpl(private val configuration: DeserializationConfigu if (!configuration.readDeserializedContracts) return null - val worker = ContractDeserializationWorker(typeTable, typeDeserializer, ownerFunction) + val worker = ContractDeserializationWorker(typeTable, typeDeserializer, ownerFunction, storageManager) val contract = worker.deserializeContract(proto.contract) return ContractProviderKey to LazyContractProvider.createInitialized(contract) } @@ -50,12 +54,13 @@ class ContractDeserializerImpl(private val configuration: DeserializationConfigu private class ContractDeserializationWorker( private val typeTable: TypeTable, private val typeDeserializer: TypeDeserializer, - private val ownerFunction: FunctionDescriptor + private val ownerFunction: FunctionDescriptor, + private val storageManager: StorageManager ) { fun deserializeContract(proto: ProtoBuf.Contract): ContractDescription? { val effects = proto.effectList.map { deserializePossiblyConditionalEffect(it) ?: return null } - return ContractDescription(effects, ownerFunction) + return ContractDescription(effects, ownerFunction, storageManager) } private fun deserializePossiblyConditionalEffect(proto: ProtoBuf.Effect): EffectDeclaration? { @@ -75,7 +80,7 @@ class ContractDeserializerImpl(private val configuration: DeserializationConfigu val argument = proto.effectConstructorArgumentList.getOrNull(0) val returnValue = if (argument == null) ConstantReference.WILDCARD else deserializeExpression(argument) as? ConstantReference - ?: return null + ?: return null ReturnsEffectDeclaration(returnValue) } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt index 6d126942c58..14fcbf1c16b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/EffectsExtractingVisitor.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.contracts import org.jetbrains.kotlin.builtins.DefaultBuiltIns +import org.jetbrains.kotlin.contracts.description.ContractProviderKey import org.jetbrains.kotlin.contracts.interpretation.ContractInterpretationDispatcher import org.jetbrains.kotlin.contracts.model.Computation import org.jetbrains.kotlin.contracts.model.ConditionalEffect @@ -168,11 +169,8 @@ class EffectsExtractingVisitor( } private fun FunctionDescriptor.getFunctor(): Functor? { - trace[BindingContext.FUNCTOR, this]?.let { return it } - - val functor = ContractInterpretationDispatcher().resolveFunctor(this) ?: return null - trace.record(BindingContext.FUNCTOR, this, functor) - return functor + val contractDescription = getUserData(ContractProviderKey)?.getContractDescription() ?: return null + return contractDescription.functor } private fun ResolvedCall<*>.isCallWithUnsupportedReceiver(): Boolean = diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt index 19a0853eb7e..a8469b90f30 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt @@ -32,8 +32,9 @@ import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.scopes.LexicalScope import org.jetbrains.kotlin.resolve.scopes.LexicalScopeKind +import org.jetbrains.kotlin.storage.StorageManager -class ContractParsingServices(val languageVersionSettings: LanguageVersionSettings) { +class ContractParsingServices(val languageVersionSettings: LanguageVersionSettings, private val storageManager: StorageManager) { /** * ! IMPORTANT NOTICE ! * @@ -77,7 +78,7 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin // Small optimization: do not even try to parse contract if we already have errors if (collector.hasErrors()) return null - val parsedContract = PsiContractParserDispatcher(collector, callContext).parseContract() + val parsedContract = PsiContractParserDispatcher(collector, callContext, storageManager).parseContract() // Make sure that at least generic error will be reported if we couldn't parse contract // (null returned => at least one error was reported) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/PsiContractParserDispatcher.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/PsiContractParserDispatcher.kt index 01c5eb0098d..a68babb5eff 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/PsiContractParserDispatcher.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/PsiContractParserDispatcher.kt @@ -40,10 +40,12 @@ import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtLambdaExpression import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.callUtil.getType +import org.jetbrains.kotlin.storage.StorageManager internal class PsiContractParserDispatcher( private val collector: ContractParsingDiagnosticsCollector, - private val callContext: ContractCallContext + private val callContext: ContractCallContext, + private val storageManager: StorageManager ) { private val conditionParser = PsiConditionParser(collector, callContext, this) private val constantParser = PsiConstantParser(callContext) @@ -71,7 +73,7 @@ internal class PsiContractParserDispatcher( if (effects.isEmpty()) return null - return ContractDescription(effects, callContext.functionDescriptor) + return ContractDescription(effects, callContext.functionDescriptor, storageManager) } fun parseCondition(expression: KtExpression?): BooleanExpression? = expression?.accept(conditionParser, Unit) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java index 371a2801419..363f8cd67a7 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java @@ -98,7 +98,6 @@ public interface BindingContext { WritableSlice DATA_FLOW_INFO_BEFORE = new BasicWritableSlice<>(DO_NOTHING); WritableSlice EXPECTED_EXPRESSION_TYPE = new BasicWritableSlice<>(DO_NOTHING); WritableSlice EXPRESSION_EFFECTS = Slices.createSimpleSlice(); - WritableSlice FUNCTOR = Slices.createSimpleSlice(); WritableSlice EXPECTED_RETURN_TYPE = new BasicWritableSlice<>(DO_NOTHING); WritableSlice DATAFLOW_INFO_AFTER_CONDITION = Slices.createSimpleSlice(); WritableSlice BOUND_INITIALIZER_VALUE = Slices.createSimpleSlice(); diff --git a/compiler/resolution/src/org/jetbrains/kotlin/contracts/description/ContractDescription.kt b/compiler/resolution/src/org/jetbrains/kotlin/contracts/description/ContractDescription.kt index e606f15898f..65624b94088 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/contracts/description/ContractDescription.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/contracts/description/ContractDescription.kt @@ -16,7 +16,10 @@ package org.jetbrains.kotlin.contracts.description +import org.jetbrains.kotlin.contracts.interpretation.ContractInterpretationDispatcher +import org.jetbrains.kotlin.contracts.model.Functor import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.storage.StorageManager /** * This is actual model of contracts, i.e. what is expected to be parsed from @@ -30,7 +33,18 @@ import org.jetbrains.kotlin.descriptors.FunctionDescriptor * backward compatibility. Ideally, this model should only be extended, but not * changed. */ -open class ContractDescription(val effects: List, val ownerFunction: FunctionDescriptor) +open class ContractDescription( + val effects: List, + val ownerFunction: FunctionDescriptor, + storageManager: StorageManager +) { + private val functorLazyValue = storageManager.createNullableLazyValue { + ContractInterpretationDispatcher().convertContractDescriptorToFunctor(this) + } + + val functor: Functor? + get() = functorLazyValue() +} interface ContractDescriptionElement { fun accept(contractDescriptionVisitor: ContractDescriptionVisitor, data: D): R diff --git a/compiler/resolution/src/org/jetbrains/kotlin/contracts/interpretation/ContractInterpretationDispatcher.kt b/compiler/resolution/src/org/jetbrains/kotlin/contracts/interpretation/ContractInterpretationDispatcher.kt index 2e2db4d8752..9e09e07879f 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/contracts/interpretation/ContractInterpretationDispatcher.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/contracts/interpretation/ContractInterpretationDispatcher.kt @@ -39,12 +39,7 @@ class ContractInterpretationDispatcher { CallsEffectInterpreter(this) ) - fun resolveFunctor(functionDescriptor: FunctionDescriptor): Functor? { - val contractDescriptor = functionDescriptor.getUserData(ContractProviderKey)?.getContractDescription() ?: return null - return convertContractDescriptorToFunctor(contractDescriptor) - } - - private fun convertContractDescriptorToFunctor(contractDescription: ContractDescription): Functor? { + fun convertContractDescriptorToFunctor(contractDescription: ContractDescription): Functor? { val resultingClauses = contractDescription.effects.map { effect -> if (effect is ConditionalEffectDeclaration) { conditionalEffectInterpreter.interpret(effect) ?: return null diff --git a/compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/rewriteAtSliceFunctor.txt b/compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/rewriteAtSliceFunctor.txt index 59f87deea94..3799072dadf 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/rewriteAtSliceFunctor.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/rewriteAtSliceFunctor.txt @@ -1,10 +1,10 @@ package public final class Foo { - public constructor Foo() - private final val foo: Foo.foo. - public final var property: kotlin.String? - public final inline fun equalVetoingObservable(/*0*/ initialValue: T, /*1*/ crossinline onChange: (newValue: T) -> kotlin.Unit): kotlin.properties.ObservableProperty + public constructor Foo(/*0*/ str: kotlin.String?) + public final val first: kotlin.Boolean + public final val second: kotlin.Boolean + public final val str: kotlin.String? public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/classFile/DeserializerForClassfileDecompiler.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/classFile/DeserializerForClassfileDecompiler.kt index 86dddc81272..fab6e865745 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/classFile/DeserializerForClassfileDecompiler.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/classFile/DeserializerForClassfileDecompiler.kt @@ -77,7 +77,7 @@ class DeserializerForClassfileDecompiler( storageManager, moduleDescriptor, configuration, classDataFinder, annotationAndConstantLoader, packageFragmentProvider, ResolveEverythingToKotlinAnyLocalClassifierResolver(builtIns), LoggingErrorReporter(LOG), LookupTracker.DO_NOTHING, JavaFlexibleTypeDeserializer, emptyList(), notFoundClasses, - ContractDeserializerImpl(configuration), + ContractDeserializerImpl(configuration, storageManager), extensionRegistryLite = JvmProtoBufUtil.EXTENSION_REGISTRY ) } diff --git a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/kotlinJavascriptPackageFragmentProvider.kt b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/kotlinJavascriptPackageFragmentProvider.kt index 9ceee7ead02..6c041d1daa4 100644 --- a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/kotlinJavascriptPackageFragmentProvider.kt +++ b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/kotlinJavascriptPackageFragmentProvider.kt @@ -76,7 +76,7 @@ fun createKotlinJavascriptPackageFragmentProvider( DynamicTypeDeserializer, emptyList(), notFoundClasses, - ContractDeserializerImpl(configuration), + ContractDeserializerImpl(configuration, storageManager), platformDependentDeclarationFilter = PlatformDependentDeclarationFilter.NoPlatformDependent, extensionRegistryLite = JsSerializerProtocol.extensionRegistry ) diff --git a/konan/library-reader/src/org/jetbrains/kotlin/serialization/konan/impl/KonanDeserializedPackageFragmentsFactoryImpl.kt b/konan/library-reader/src/org/jetbrains/kotlin/serialization/konan/impl/KonanDeserializedPackageFragmentsFactoryImpl.kt index 59b36d2442f..f50bb5bfed5 100644 --- a/konan/library-reader/src/org/jetbrains/kotlin/serialization/konan/impl/KonanDeserializedPackageFragmentsFactoryImpl.kt +++ b/konan/library-reader/src/org/jetbrains/kotlin/serialization/konan/impl/KonanDeserializedPackageFragmentsFactoryImpl.kt @@ -119,7 +119,7 @@ internal object KonanDeserializedPackageFragmentsFactoryImpl : KonanDeserialized NullFlexibleTypeDeserializer, emptyList(), notFoundClasses, - ContractDeserializerImpl(configuration), + ContractDeserializerImpl(configuration, storageManager), extensionRegistryLite = KonanSerializerProtocol.extensionRegistry )