From 0e77b671705bb97bfec865bdbe52df2bce20fc85 Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Thu, 9 Mar 2023 11:05:22 +0100 Subject: [PATCH] K2: implement correct check for ctors mapped from java so the check is functionally the same as in K1. #KT-57064 fixed #KT-57065 fixed One of the tests introduced here (javaMappedCtors) revealed an additional issue, filed as KT-57368 --- .../scopes/memberScopeByFqName/Int.pretty.txt | 2 - .../scopes/memberScopeByFqName/Int.txt | 44 ----------------- ...CompilerTestFE10TestdataTestGenerated.java | 12 +++++ ...eeOldFrontendDiagnosticsTestGenerated.java | 12 +++++ ...siOldFrontendDiagnosticsTestGenerated.java | 12 +++++ .../kotlin/fir/scopes/jvm/JvmMappedScope.kt | 48 ++++++++++++++----- .../platformTypes/javaMappedCtors.fir.kt | 36 ++++++++++++++ .../tests/platformTypes/javaMappedCtors.kt | 36 ++++++++++++++ .../diagnostics/tests/typealias/kt57065.kt | 8 ++++ .../test/runners/DiagnosticTestGenerated.java | 12 +++++ 10 files changed, 165 insertions(+), 57 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt create mode 100644 compiler/testData/diagnostics/tests/typealias/kt57065.kt diff --git a/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.pretty.txt b/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.pretty.txt index f33a2198d43..3707ae1b994 100644 --- a/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.pretty.txt +++ b/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.pretty.txt @@ -198,6 +198,4 @@ open fun hashCode(): kotlin.Int companion object -constructor(value: kotlin.Int) - private constructor() \ No newline at end of file diff --git a/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.txt b/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.txt index a2355da36dd..3ff5ba86b61 100644 --- a/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.txt +++ b/analysis/analysis-api/testData/scopes/memberScopeByFqName/Int.txt @@ -3602,50 +3602,6 @@ KtNamedClassOrObjectSymbol: annotationApplicableTargets: null deprecationStatus: null -KtConstructorSymbol: - annotationsList: [] - callableIdIfNonLocal: null - containingClassIdIfNonLocal: kotlin/Int - contextReceivers: [] - hasStableParameterNames: false - isExtension: false - isPrimary: false - origin: JAVA - receiverParameter: null - returnType: KtUsualClassType: - annotationsList: [] - ownTypeArguments: [] - type: java/lang/Integer - symbolKind: CLASS_MEMBER - typeParameters: [] - valueParameters: [ - KtValueParameterSymbol: - annotationsList: [] - callableIdIfNonLocal: null - contextReceivers: [] - generatedPrimaryConstructorProperty: null - hasDefaultValue: false - isCrossinline: false - isExtension: false - isImplicitLambdaParameter: false - isNoinline: false - isVararg: false - name: value - origin: SUBSTITUTION_OVERRIDE - receiverParameter: null - returnType: KtUsualClassType: - annotationsList: [] - ownTypeArguments: [] - type: kotlin/Int - symbolKind: LOCAL - typeParameters: [] - getContainingModule: KtSdkModule "SDK jdk" - deprecationStatus: null - ] - visibility: Public - getContainingModule: KtSourceModule "Sources of main" - deprecationStatus: null - KtConstructorSymbol: annotationsList: [] callableIdIfNonLocal: null diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index bb4660ec348..8151014a261 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -24324,6 +24324,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/platformTypes/javaEmptyList.kt"); } + @Test + @TestMetadata("javaMappedCtors.kt") + public void testJavaMappedCtors() throws Exception { + runTest("compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt"); + } + @Test @TestMetadata("kt50877.kt") public void testKt50877() throws Exception { @@ -33810,6 +33816,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/typealias/kt19601.kt"); } + @Test + @TestMetadata("kt57065.kt") + public void testKt57065() throws Exception { + runTest("compiler/testData/diagnostics/tests/typealias/kt57065.kt"); + } + @Test @TestMetadata("localTypeAlias.kt") public void testLocalTypeAlias() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index 607b3e59d96..d49dbf74338 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -24324,6 +24324,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/platformTypes/javaEmptyList.kt"); } + @Test + @TestMetadata("javaMappedCtors.kt") + public void testJavaMappedCtors() throws Exception { + runTest("compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt"); + } + @Test @TestMetadata("kt50877.kt") public void testKt50877() throws Exception { @@ -33810,6 +33816,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/typealias/kt19601.kt"); } + @Test + @TestMetadata("kt57065.kt") + public void testKt57065() throws Exception { + runTest("compiler/testData/diagnostics/tests/typealias/kt57065.kt"); + } + @Test @TestMetadata("localTypeAlias.kt") public void testLocalTypeAlias() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index 5adbc10716e..e1c2ddff890 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -24330,6 +24330,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/platformTypes/javaEmptyList.kt"); } + @Test + @TestMetadata("javaMappedCtors.kt") + public void testJavaMappedCtors() throws Exception { + runTest("compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt"); + } + @Test @TestMetadata("kt50877.kt") public void testKt50877() throws Exception { @@ -33906,6 +33912,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/typealias/kt19601.kt"); } + @Test + @TestMetadata("kt57065.kt") + public void testKt57065() throws Exception { + runTest("compiler/testData/diagnostics/tests/typealias/kt57065.kt"); + } + @Test @TestMetadata("localTypeAlias.kt") public void testLocalTypeAlias() throws Exception { diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/scopes/jvm/JvmMappedScope.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/scopes/jvm/JvmMappedScope.kt index 71971e32e67..a96bb794dfa 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/scopes/jvm/JvmMappedScope.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/scopes/jvm/JvmMappedScope.kt @@ -8,7 +8,9 @@ package org.jetbrains.kotlin.fir.scopes.jvm import org.jetbrains.kotlin.builtins.jvm.JvmBuiltInsSignatures import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.declarations.FirClass +import org.jetbrains.kotlin.fir.declarations.FirConstructor import org.jetbrains.kotlin.fir.declarations.FirRegularClass +import org.jetbrains.kotlin.fir.declarations.constructors import org.jetbrains.kotlin.fir.declarations.utils.classId import org.jetbrains.kotlin.fir.resolve.defaultType import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor @@ -17,11 +19,12 @@ import org.jetbrains.kotlin.fir.scopes.FirContainingNamesAwareScope import org.jetbrains.kotlin.fir.scopes.FirTypeScope import org.jetbrains.kotlin.fir.scopes.ProcessorAction import org.jetbrains.kotlin.fir.scopes.impl.FirFakeOverrideGenerator +import org.jetbrains.kotlin.fir.scopes.impl.FirStandardOverrideChecker +import org.jetbrains.kotlin.fir.scopes.impl.buildSubstitutorForOverridesCheck import org.jetbrains.kotlin.fir.symbols.ConeTypeParameterLookupTag import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.fir.types.coneType import org.jetbrains.kotlin.fir.types.impl.ConeTypeParameterTypeImpl -import org.jetbrains.kotlin.load.kotlin.SignatureBuildingComponents.inJavaLang import org.jetbrains.kotlin.name.CallableId import org.jetbrains.kotlin.name.Name @@ -37,6 +40,8 @@ class JvmMappedScope( private val constructorsCache = mutableMapOf() + private val overrideChecker = FirStandardOverrideChecker(session) + private val substitutor = ConeSubstitutorByMap( firJavaClass.typeParameters.zip(firKotlinClass.typeParameters).associate { (javaParameter, kotlinParameter) -> javaParameter.symbol to ConeTypeParameterTypeImpl(ConeTypeParameterLookupTag(kotlinParameter.symbol), isNullable = false) @@ -96,19 +101,40 @@ class JvmMappedScope( processor: (FirNamedFunctionSymbol, FirTypeScope) -> ProcessorAction ) = ProcessorAction.NONE + private val firKotlinClassConstructors by lazy(LazyThreadSafetyMode.PUBLICATION) { + firKotlinClass.constructors(session) + } + override fun processDeclaredConstructors(processor: (FirConstructorSymbol) -> Unit) { - val hiddenConstructors = signatures.hiddenConstructors - if (hiddenConstructors.isNotEmpty()) { - javaMappedClassUseSiteScope.processDeclaredConstructors { symbol -> - val jvmSignature = symbol.fir.computeJvmDescriptor() - if (jvmSignature !in hiddenConstructors) { - val newSymbol = getOrCreateCopy(symbol) - processor(newSymbol) + javaMappedClassUseSiteScope.processDeclaredConstructors { javaCtorSymbol -> + + fun FirConstructor.isShadowedBy(ctorFromKotlin: FirConstructorSymbol): Boolean { + // assuming already checked for visibility + val valueParams = valueParameters + val valueParamsFromKotlin = ctorFromKotlin.fir.valueParameters + if (valueParams.size != valueParamsFromKotlin.size) return false + val substitutor = buildSubstitutorForOverridesCheck(ctorFromKotlin.fir, this@isShadowedBy, session) ?: return false + return valueParamsFromKotlin.zip(valueParams).all { (kotlinCtorParam, javaCtorParam) -> + overrideChecker.isEqualTypes(kotlinCtorParam.returnTypeRef, javaCtorParam.returnTypeRef, substitutor) } } - } else { - javaMappedClassUseSiteScope.processDeclaredConstructors { symbol -> - val newSymbol = getOrCreateCopy(symbol) + + fun FirConstructor.isTrivialCopyConstructor(): Boolean = + valueParameters.singleOrNull()?.let { + val type = substitutor.substituteOrSelf(it.returnTypeRef.coneType) + type == firKotlinClass.defaultType() + } ?: false + + // In K1 it is handled by JvmBuiltInsCustomizer.getConstructors + // Here the logic is generally the same, but simplified for performance by reordering checks and avoiding checking + // for the impossible combinations + val javaCtor = javaCtorSymbol.fir + if (javaCtor.status.visibility.isPublicAPI && + javaCtor.computeJvmDescriptor() !in signatures.hiddenConstructors && + !javaCtor.isTrivialCopyConstructor() && + firKotlinClassConstructors.none { javaCtor.isShadowedBy(it) } + ) { + val newSymbol = getOrCreateCopy(javaCtorSymbol) processor(newSymbol) } } diff --git a/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt new file mode 100644 index 00000000000..0139afd7672 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.fir.kt @@ -0,0 +1,36 @@ +// TARGET_BACKEND: JVM_IR + +typealias ABoolean = Boolean +typealias AChar = Char +typealias AInt = Int +typealias ALong = Long +typealias AShort = Short +typealias AByte = Byte +typealias AFloat = Float +typealias ADouble = Double + +fun main() { + ABoolean(false) + Boolean(false) + + AChar('c') + Char('c') + + AInt(42) + Int(42) + + ALong(42) + Long(42) + + AShort(42) + Short(42) + + AByte(42) + Byte(42) + + AFloat(4.2f) + Float(4.2f) + + ADouble(4.2) + Double(4.2) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt new file mode 100644 index 00000000000..99a35b39dea --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt @@ -0,0 +1,36 @@ +// TARGET_BACKEND: JVM_IR + +typealias ABoolean = Boolean +typealias AChar = Char +typealias AInt = Int +typealias ALong = Long +typealias AShort = Short +typealias AByte = Byte +typealias AFloat = Float +typealias ADouble = Double + +fun main() { + ABoolean(false) + Boolean(false) + + AChar('c') + Char('c') + + AInt(42) + Int(42) + + ALong(42) + Long(42) + + AShort(42) + Short(42) + + AByte(42) + Byte(42) + + AFloat(4.2f) + Float(4.2f) + + ADouble(4.2) + Double(4.2) +} diff --git a/compiler/testData/diagnostics/tests/typealias/kt57065.kt b/compiler/testData/diagnostics/tests/typealias/kt57065.kt new file mode 100644 index 00000000000..815cc7a4c23 --- /dev/null +++ b/compiler/testData/diagnostics/tests/typealias/kt57065.kt @@ -0,0 +1,8 @@ +// FIR_IDENTICAL + +typealias GlobalUndoLogRef = Long +fun GlobalUndoLogRef(p: Long): GlobalUndoLogRef = p + +fun main() { + GlobalUndoLogRef(42) +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 09bebe26781..30d15b86549 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -24330,6 +24330,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/platformTypes/javaEmptyList.kt"); } + @Test + @TestMetadata("javaMappedCtors.kt") + public void testJavaMappedCtors() throws Exception { + runTest("compiler/testData/diagnostics/tests/platformTypes/javaMappedCtors.kt"); + } + @Test @TestMetadata("kt50877.kt") public void testKt50877() throws Exception { @@ -33906,6 +33912,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/typealias/kt19601.kt"); } + @Test + @TestMetadata("kt57065.kt") + public void testKt57065() throws Exception { + runTest("compiler/testData/diagnostics/tests/typealias/kt57065.kt"); + } + @Test @TestMetadata("localTypeAlias.kt") public void testLocalTypeAlias() throws Exception {