From bced604b57c17536e70f278fb14c3eb42738b1e6 Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Tue, 24 Oct 2023 15:00:01 +0300 Subject: [PATCH] [FIR2IR] Remove incorrect usages of Fir2IrDeclarationStorage.getOrCreateIrFunction `getOrCreateXxx` methods have not very safe semantics: they sometimes create an IR declaration if it was missing in caches before. For all source declarations, this is not very good semantic, because we have strict rules in which order source declarations should be created, and usage of `getOrCreateXxx` may break this order. So it's important to be able to track all invocations of such methods to ensure that it's appropriate in each case. To achieve that, a new `@GetOrCreateSensitiveAPI ` opt-in is introduced --- .../jetbrains/kotlin/fir/backend/Fir2IrConverter.kt | 5 ++++- .../kotlin/fir/backend/Fir2IrDeclarationStorage.kt | 11 ++++++++++- .../org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt | 4 ++-- .../org/jetbrains/kotlin/fir/backend/FirIrProvider.kt | 1 + .../jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt | 1 + .../backend/generators/DelegatedMemberGenerator.kt | 2 +- .../org/jetbrains/kotlin/fir/lazy/Fir2IrLazyClass.kt | 1 + .../kotlin/fir/lazy/Fir2IrLazyPropertyAccessor.kt | 1 + .../kotlin/fir/lazy/Fir2IrLazySimpleFunction.kt | 2 ++ 9 files changed, 23 insertions(+), 5 deletions(-) diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt index defaf337dbc..e5395201e4c 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrConverter.kt @@ -458,6 +458,9 @@ class Fir2IrConverter( } } + /** + * This function creates IR declarations for callable members without filling their body + */ private fun processMemberDeclaration( declaration: FirDeclaration, containingClass: FirClass?, @@ -508,7 +511,7 @@ class Fir2IrConverter( } } is FirSimpleFunction -> { - declarationStorage.getOrCreateIrFunction(declaration, parent, isLocal = isInLocalClass) + declarationStorage.createAndCacheIrFunction(declaration, parent, isLocal = isInLocalClass) } is FirProperty -> { if ( diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt index d45380ea045..118f0cb5d72 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrDeclarationStorage.kt @@ -269,6 +269,7 @@ class Fir2IrDeclarationStorage( return cachedIrCallable } + @GetOrCreateSensitiveAPI fun getOrCreateIrFunction( function: FirFunction, irParent: IrDeclarationParent?, @@ -279,6 +280,7 @@ class Fir2IrDeclarationStorage( return getOrCreateIrFunction(function, { irParent }, predefinedOrigin, isLocal, fakeOverrideOwnerLookupTag) } + @GetOrCreateSensitiveAPI private fun getOrCreateIrFunction( function: FirFunction, irParent: () -> IrDeclarationParent?, @@ -301,7 +303,6 @@ class Fir2IrDeclarationStorage( return createAndCacheIrFunction(function, irParent(), predefinedOrigin, isLocal, fakeOverrideOwnerLookupTag) } - @LeakedDeclarationCaches fun createAndCacheIrFunction( function: FirFunction, irParent: IrDeclarationParent?, @@ -874,6 +875,7 @@ class Fir2IrDeclarationStorage( getIrConstructorSymbol(fir.symbol) } else -> { + @OptIn(GetOrCreateSensitiveAPI::class) getOrCreateIrFunction( fir, { findIrParent(fir, fakeOverrideOwnerLookupTag) }, @@ -1177,6 +1179,13 @@ internal var FirProperty.isStubPropertyForPureField: Boolean? by FirDeclarationD @RequiresOptIn annotation class LeakedDeclarationCaches +/** + * This annotation indicates that an annotated method can create a declaration in ad-hock way, so it should be used with caution + * In most cases, it's recommended to use method which return symbol or which forcefully creates a declaration + */ +@RequiresOptIn +annotation class GetOrCreateSensitiveAPI + @OptIn(UnsafeDuringIrConstructionAPI::class) internal fun IrBindableSymbol<*, D>.ownerIfBound(): D? { return runIf(isBound) { owner } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt index 37246c26392..d3310733423 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt @@ -401,7 +401,7 @@ class Fir2IrVisitor( override fun visitSimpleFunction(simpleFunction: FirSimpleFunction, data: Any?): IrElement = whileAnalysing(session, simpleFunction) { val irFunction = if (simpleFunction.visibility == Visibilities.Local) { - declarationStorage.getOrCreateIrFunction( + declarationStorage.createAndCacheIrFunction( simpleFunction, irParent = conversionScope.parent(), predefinedOrigin = IrDeclarationOrigin.LOCAL_FUNCTION, isLocal = true ) } else { @@ -424,7 +424,7 @@ class Fir2IrVisitor( data: Any? ): IrElement = whileAnalysing(session, anonymousFunction) { return anonymousFunction.convertWithOffsets { startOffset, endOffset -> - val irFunction = declarationStorage.getOrCreateIrFunction( + val irFunction = declarationStorage.createAndCacheIrFunction( anonymousFunction, irParent = conversionScope.parent(), predefinedOrigin = IrDeclarationOrigin.LOCAL_FUNCTION, diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt index 98fc1fbfa15..3bc6f74b032 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/FirIrProvider.kt @@ -173,6 +173,7 @@ class FirIrProvider(val components: Fir2IrComponents) : IrProvider { } SymbolKind.FUNCTION_SYMBOL -> { val firSimpleFunction = firDeclaration as FirSimpleFunction + @OptIn(GetOrCreateSensitiveAPI::class) declarationStorage.getOrCreateIrFunction(firSimpleFunction, parent) } SymbolKind.PROPERTY_SYMBOL -> { diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt index d2e1513cdbd..3f73b95441a 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/IrBuiltInsOverFir.kt @@ -698,6 +698,7 @@ class IrBuiltInsOverFir( functionSymbol.lazyResolveToPhase(FirResolvePhase.IMPLICIT_TYPES_BODY_RESOLVE) val irParent = findIrParent(functionSymbol) + @OptIn(GetOrCreateSensitiveAPI::class) return components.declarationStorage.getOrCreateIrFunction(functionSymbol.fir, irParent).symbol } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/DelegatedMemberGenerator.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/DelegatedMemberGenerator.kt index 045929c96c9..9a2597f8ff2 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/DelegatedMemberGenerator.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/DelegatedMemberGenerator.kt @@ -214,7 +214,7 @@ class DelegatedMemberGenerator(private val components: Fir2IrComponents) : Fir2I firSubClass: FirClass, delegateOverride: FirSimpleFunction ): IrSimpleFunction { - val delegateFunction = declarationStorage.getOrCreateIrFunction( + val delegateFunction = declarationStorage.createAndCacheIrFunction( delegateOverride, subClass, predefinedOrigin = IrDeclarationOrigin.DELEGATED_MEMBER, fakeOverrideOwnerLookupTag = firSubClass.symbol.toLookupTag() ) diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyClass.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyClass.kt index 07f14bc41c0..235f033ef24 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyClass.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyClass.kt @@ -195,6 +195,7 @@ class Fir2IrLazyClass( symbol.containingClassLookupTag() != ownerLookupTag -> {} symbol.isAbstractMethodOfAny() -> {} else -> { + @OptIn(GetOrCreateSensitiveAPI::class) result += declarationStorage.getOrCreateIrFunction(symbol.fir, this, origin) } } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyPropertyAccessor.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyPropertyAccessor.kt index 80cc5547365..046121f5973 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyPropertyAccessor.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazyPropertyAccessor.kt @@ -125,6 +125,7 @@ class Fir2IrLazyPropertyAccessor( override val initialSignatureFunction: IrFunction? by lazy { val originalFirFunction = (fir as? FirSyntheticPropertyAccessor)?.delegate ?: return@lazy null + @OptIn(GetOrCreateSensitiveAPI::class) declarationStorage.getOrCreateIrFunction(originalFirFunction, parent) } diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazySimpleFunction.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazySimpleFunction.kt index 209f4a5a906..a87555a93b7 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazySimpleFunction.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/lazy/Fir2IrLazySimpleFunction.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.fir.lazy import org.jetbrains.kotlin.fir.backend.Fir2IrComponents +import org.jetbrains.kotlin.fir.backend.GetOrCreateSensitiveAPI import org.jetbrains.kotlin.fir.backend.contextReceiversForFunctionOrContainingProperty import org.jetbrains.kotlin.fir.backend.generators.Fir2IrCallableDeclarationsGenerator import org.jetbrains.kotlin.fir.backend.generators.generateOverriddenFunctionSymbols @@ -110,6 +111,7 @@ class Fir2IrLazySimpleFunction( override val initialSignatureFunction: IrFunction? by lazy { val originalFunction = fir.initialSignatureAttr as? FirFunction ?: return@lazy null + @OptIn(GetOrCreateSensitiveAPI::class) declarationStorage.getOrCreateIrFunction(originalFunction, parent).also { check(it !== this) { "Initial function can not be the same as remapped function" } }