From 106ee1d1ffd582a4f57b82ddac41d25e521ab835 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 2 Nov 2020 18:32:01 +0100 Subject: [PATCH] JVM IR: restore ability to suppress codegen-reported diagnostics In the old backend, BindingContextSuppressCache is used (which is now created explicitly in GenerationState), which looks up `@Suppress` annotations on elements right before reporting the diagnostic. In JVM IR, we clear the binding context after psi2ir, so this approach doesn't work. This change provides another implementation of KotlinSuppressCache which eagerly precomputes all suppressions on all annotated elements in all source files at the point of creation of GenerationState (when the binding context is still full). #KT-43047 Fixed --- .../kotlin/codegen/state/GenerationState.kt | 7 ++- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 ++ .../kotlin/resolve/BindingTraceContext.java | 9 +-- .../kotlin/resolve/DelegatingBindingTrace.kt | 20 +++---- .../kotlin/resolve/TemporaryBindingTrace.java | 2 +- .../DiagnosticsWithSuppression.java | 13 ++-- .../MutableDiagnosticsWithSuppression.kt | 20 +++---- .../diagnostics/PrecomputedSuppressCache.kt | 60 +++++++++++++++++++ .../ir/suppressConflictingSignatureErrors.kt | 25 ++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ 12 files changed, 142 insertions(+), 34 deletions(-) create mode 100644 compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/PrecomputedSuppressCache.kt create mode 100644 compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt index de011536058..1d99b53b92d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt @@ -39,6 +39,7 @@ import org.jetbrains.kotlin.resolve.* import org.jetbrains.kotlin.resolve.deprecation.CoroutineCompatibilitySupport import org.jetbrains.kotlin.resolve.deprecation.DeprecationResolver import org.jetbrains.kotlin.resolve.diagnostics.Diagnostics +import org.jetbrains.kotlin.resolve.diagnostics.PrecomputedSuppressCache import org.jetbrains.kotlin.resolve.jvm.JvmClassName import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKind.* @@ -178,7 +179,11 @@ class GenerationState private constructor( } val extraJvmDiagnosticsTrace: BindingTrace = - DelegatingBindingTrace(originalFrontendBindingContext, "For extra diagnostics in ${this::class.java}", false) + DelegatingBindingTrace( + originalFrontendBindingContext, "For extra diagnostics in ${this::class.java}", false, + customSuppressCache = if (isIrBackend) PrecomputedSuppressCache(originalFrontendBindingContext, files) else null, + ) + private val interceptedBuilderFactory: ClassBuilderFactory private var used = false diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index c5e0a449473..df645ee4e63 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -15807,6 +15807,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/ir/simple.kt"); } + @TestMetadata("suppressConflictingSignatureErrors.kt") + public void testSuppressConflictingSignatureErrors() throws Exception { + runTest("compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt"); + } + @TestMetadata("compiler/testData/codegen/box/ir/closureConversion") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java index 0d97fc986c7..49946047706 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java @@ -22,6 +22,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; import org.jetbrains.kotlin.diagnostics.Diagnostic; import org.jetbrains.kotlin.psi.KtExpression; +import org.jetbrains.kotlin.resolve.diagnostics.BindingContextSuppressCache; import org.jetbrains.kotlin.resolve.diagnostics.Diagnostics; import org.jetbrains.kotlin.resolve.diagnostics.MutableDiagnosticsWithSuppression; import org.jetbrains.kotlin.types.KotlinType; @@ -93,12 +94,12 @@ public class BindingTraceContext implements BindingTrace { this(TRACK_REWRITES && !allowSliceRewrite ? new TrackingSlicedMap(TRACK_WITH_STACK_TRACES) : new SlicedMapImpl(allowSliceRewrite), filter); } - private BindingTraceContext(@NotNull MutableSlicedMap map, BindingTraceFilter filter) { this.map = map; - this.mutableDiagnostics = !filter.getIgnoreDiagnostics() - ? new MutableDiagnosticsWithSuppression(bindingContext, Diagnostics.Companion.getEMPTY()) - : null; + this.mutableDiagnostics = + filter.getIgnoreDiagnostics() + ? null + : new MutableDiagnosticsWithSuppression(new BindingContextSuppressCache(bindingContext), Diagnostics.Companion.getEMPTY()); } @TestOnly diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatingBindingTrace.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatingBindingTrace.kt index 3b284d77a93..59c1c93dc27 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatingBindingTrace.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatingBindingTrace.kt @@ -20,7 +20,9 @@ import com.google.common.collect.ImmutableMap import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.resolve.diagnostics.BindingContextSuppressCache import org.jetbrains.kotlin.resolve.diagnostics.Diagnostics +import org.jetbrains.kotlin.resolve.diagnostics.KotlinSuppressCache import org.jetbrains.kotlin.resolve.diagnostics.MutableDiagnosticsWithSuppression import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.expressions.typeInfoFactory.createTypeInfo @@ -31,7 +33,8 @@ open class DelegatingBindingTrace( private val name: String, withParentDiagnostics: Boolean = true, private val filter: BindingTraceFilter = BindingTraceFilter.ACCEPT_ALL, - allowSliceRewrite: Boolean = false + allowSliceRewrite: Boolean = false, + customSuppressCache: KotlinSuppressCache? = null, ) : BindingTrace { protected val map = if (BindingTraceContext.TRACK_REWRITES && !allowSliceRewrite) @@ -39,8 +42,6 @@ open class DelegatingBindingTrace( else SlicedMapImpl(allowSliceRewrite) - protected val mutableDiagnostics: MutableDiagnosticsWithSuppression? - private inner class MyBindingContext : BindingContext { override fun getDiagnostics(): Diagnostics = mutableDiagnostics ?: Diagnostics.EMPTY @@ -68,13 +69,12 @@ open class DelegatingBindingTrace( private val bindingContext = MyBindingContext() - init { - this.mutableDiagnostics = when { - filter.ignoreDiagnostics -> null - withParentDiagnostics -> MutableDiagnosticsWithSuppression(bindingContext, parentContext.diagnostics) - else -> MutableDiagnosticsWithSuppression(bindingContext) - } - } + protected val mutableDiagnostics: MutableDiagnosticsWithSuppression? = + if (filter.ignoreDiagnostics) null + else MutableDiagnosticsWithSuppression( + customSuppressCache ?: BindingContextSuppressCache(bindingContext), + if (withParentDiagnostics) parentContext.diagnostics else Diagnostics.EMPTY + ) constructor( parentContext: BindingContext, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TemporaryBindingTrace.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TemporaryBindingTrace.java index 1c2d37ccc1e..a47b22985f1 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/TemporaryBindingTrace.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/TemporaryBindingTrace.java @@ -39,7 +39,7 @@ public class TemporaryBindingTrace extends DelegatingBindingTrace { protected final BindingTrace trace; protected TemporaryBindingTrace(@NotNull BindingTrace trace, String debugName, BindingTraceFilter filter) { - super(trace.getBindingContext(), debugName, true, filter, false); + super(trace.getBindingContext(), debugName, true, filter, false, null); this.trace = trace; } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/DiagnosticsWithSuppression.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/DiagnosticsWithSuppression.java index 4034b27f304..4463565bd84 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/DiagnosticsWithSuppression.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/DiagnosticsWithSuppression.java @@ -23,20 +23,19 @@ import kotlin.collections.CollectionsKt; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.TestOnly; import org.jetbrains.kotlin.diagnostics.Diagnostic; -import org.jetbrains.kotlin.resolve.BindingContext; import java.util.Collection; import java.util.Iterator; public class DiagnosticsWithSuppression implements Diagnostics { - private final KotlinSuppressCache kotlinSuppressCache; + private final KotlinSuppressCache suppressCache; private final Collection diagnostics; private final DiagnosticsElementsCache elementsCache; - public DiagnosticsWithSuppression(@NotNull BindingContext context, @NotNull Collection diagnostics) { + public DiagnosticsWithSuppression(@NotNull KotlinSuppressCache suppressCache, @NotNull Collection diagnostics) { this.diagnostics = diagnostics; - this.kotlinSuppressCache = new BindingContextSuppressCache(context); - this.elementsCache = new DiagnosticsElementsCache(this, kotlinSuppressCache.getFilter()); + this.suppressCache = suppressCache; + this.elementsCache = new DiagnosticsElementsCache(this, suppressCache.getFilter()); } @NotNull @@ -48,13 +47,13 @@ public class DiagnosticsWithSuppression implements Diagnostics { @NotNull @Override public Iterator iterator() { - return new FilteringIterator<>(diagnostics.iterator(), kotlinSuppressCache.getFilter()::invoke); + return new FilteringIterator<>(diagnostics.iterator(), suppressCache.getFilter()::invoke); } @NotNull @Override public Collection all() { - return CollectionsKt.filter(diagnostics, kotlinSuppressCache.getFilter()); + return CollectionsKt.filter(diagnostics, suppressCache.getFilter()); } @NotNull diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt index 47b3f4934ce..7157dfdec02 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt @@ -16,26 +16,24 @@ package org.jetbrains.kotlin.resolve.diagnostics -import org.jetbrains.kotlin.diagnostics.Diagnostic -import java.util.ArrayList import com.intellij.openapi.util.CompositeModificationTracker -import com.intellij.util.CachedValueImpl -import com.intellij.psi.util.CachedValueProvider import com.intellij.psi.PsiElement +import com.intellij.psi.util.CachedValueProvider +import com.intellij.util.CachedValueImpl import org.jetbrains.annotations.TestOnly -import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.diagnostics.Diagnostic -class MutableDiagnosticsWithSuppression @JvmOverloads constructor( - private val bindingContext: BindingContext, - private val delegateDiagnostics: Diagnostics = Diagnostics.EMPTY +class MutableDiagnosticsWithSuppression( + private val suppressCache: KotlinSuppressCache, + private val delegateDiagnostics: Diagnostics, ) : Diagnostics { private val diagnosticList = ArrayList() //NOTE: CachedValuesManager is not used because it requires Project passed to this object - private val cache = CachedValueImpl(CachedValueProvider { + private val cache = CachedValueImpl { val allDiagnostics = delegateDiagnostics.noSuppression().all() + diagnosticList - CachedValueProvider.Result(DiagnosticsWithSuppression(bindingContext, allDiagnostics), modificationTracker) - }) + CachedValueProvider.Result(DiagnosticsWithSuppression(suppressCache, allDiagnostics), modificationTracker) + } private fun readonlyView(): DiagnosticsWithSuppression = cache.value!! diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/PrecomputedSuppressCache.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/PrecomputedSuppressCache.kt new file mode 100644 index 00000000000..332fa4e4225 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/PrecomputedSuppressCache.kt @@ -0,0 +1,60 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.resolve.diagnostics + +import org.jetbrains.kotlin.builtins.StandardNames +import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.psi.KtAnnotated +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtTreeVisitorVoid +import org.jetbrains.kotlin.resolve.BindingContext + +/** + * A [KotlinSuppressCache] implementation that computes all suppressions at the moment of instantiation. + * This is useful in the IR backend, where we clear the main binding context after psi2ir to avoid taking extra memory. + * To make suppression of errors reported from backend possible though, we need to precompute all resolved `@Suppress` annotations, + * and store this information outside of the binding context, which is going to be cleared. + * + * @param context the binding context where the data should be loaded from. Note that it shouldn't be stored as a property because the + * primary use case of this class is when that binding context is cleared and thus is useless after a certain point. + */ +class PrecomputedSuppressCache(context: BindingContext, files: List) : KotlinSuppressCache() { + val storage: Map> = + mutableMapOf>().also { storage -> + val visitor = PrecomputingVisitor(storage, BindingContextSuppressCache(context)) + for (file in files) { + file.accept(visitor, null) + } + } + + private class PrecomputingVisitor( + val storage: MutableMap>, + val computer: KotlinSuppressCache, + ) : KtTreeVisitorVoid() { + override fun visitKtElement(element: KtElement) { + super.visitKtElement(element) + if (element is KtAnnotated) { + computeAnnotations(element) + } + } + + override fun visitKtFile(file: KtFile) { + super.visitKtFile(file) + computeAnnotations(file) + } + + private fun computeAnnotations(element: KtAnnotated) { + val suppressions = computer.getSuppressionAnnotations(element).filter { it.fqName == StandardNames.FqNames.suppress } + if (suppressions.isNotEmpty()) { + storage[element] = suppressions + } + } + } + + override fun getSuppressionAnnotations(annotated: KtAnnotated): List = + storage[annotated].orEmpty() +} diff --git a/compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt b/compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt new file mode 100644 index 00000000000..323dfc09ec9 --- /dev/null +++ b/compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt @@ -0,0 +1,25 @@ +// This test checks that suppressing conflicting signature / accidental override errors works. +// This is used in libraries for binary-compatible migration, as well as in some cases in multiplatform projects. +// TARGET_BACKEND: JVM +// IGNORE_BACKEND_FIR: JVM_IR +// FILE: box.kt + +open class B { + open val s: String + get() = "Fail" +} + +class C : B() { + @Suppress("ACCIDENTAL_OVERRIDE") + fun getS(): String = "O" +} + +fun box(): String = C().getS() + D().getS() + +// FILE: another.kt + +@file:Suppress("ACCIDENTAL_OVERRIDE") + +class D : B() { + fun getS(): String = "K" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 82a6380bfbe..79d29e35a14 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -17207,6 +17207,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/ir/simple.kt"); } + @TestMetadata("suppressConflictingSignatureErrors.kt") + public void testSuppressConflictingSignatureErrors() throws Exception { + runTest("compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt"); + } + @TestMetadata("compiler/testData/codegen/box/ir/closureConversion") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index d2dc5b2be07..d9ede1125ca 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -17207,6 +17207,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/ir/simple.kt"); } + @TestMetadata("suppressConflictingSignatureErrors.kt") + public void testSuppressConflictingSignatureErrors() throws Exception { + runTest("compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt"); + } + @TestMetadata("compiler/testData/codegen/box/ir/closureConversion") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 5acf07f1283..416626a1a36 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -15807,6 +15807,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/ir/simple.kt"); } + @TestMetadata("suppressConflictingSignatureErrors.kt") + public void testSuppressConflictingSignatureErrors() throws Exception { + runTest("compiler/testData/codegen/box/ir/suppressConflictingSignatureErrors.kt"); + } + @TestMetadata("compiler/testData/codegen/box/ir/closureConversion") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)