Support "-Xno-check-impl" argument, check only real declarations

Use "-Xno-check-impl" to suppress checking whether the platform declaration
implementation has the "impl" modifier.

Do not check presence of fake overrides from platform class in the impl class,
otherwise there would be a lot of errors about the fact that
equals/hashCode/toString are not marked with the "impl" modifier
This commit is contained in:
Alexander Udalov
2016-11-25 17:10:46 +03:00
parent bbafb7c013
commit 204873edf2
12 changed files with 70 additions and 15 deletions
@@ -67,6 +67,9 @@ public abstract class CommonCompilerArguments {
@Argument(value = "Xmulti-platform", description = "Enable experimental language support for multi-platform projects") @Argument(value = "Xmulti-platform", description = "Enable experimental language support for multi-platform projects")
public boolean multiPlatform; public boolean multiPlatform;
@Argument(value = "Xno-check-impl", description = "Do not check presence of 'impl' modifier in multi-platform projects")
public boolean noCheckImpl;
@Argument(value = "P", description = "Pass an option to a plugin") @Argument(value = "P", description = "Pass an option to a plugin")
@ValueDescription(PLUGIN_OPTION_FORMAT) @ValueDescription(PLUGIN_OPTION_FORMAT)
public String[] pluginOptions; public String[] pluginOptions;
@@ -281,6 +281,9 @@ public abstract class CLICompiler<A extends CommonCompilerArguments> {
if (arguments.multiPlatform) { if (arguments.multiPlatform) {
extraLanguageFeatures.add(LanguageFeature.MultiPlatformProjects); extraLanguageFeatures.add(LanguageFeature.MultiPlatformProjects);
} }
if (arguments.noCheckImpl) {
extraLanguageFeatures.add(LanguageFeature.MultiPlatformDoNotCheckImpl);
}
configuration.put( configuration.put(
CommonConfigurationKeys.LANGUAGE_VERSION_SETTINGS, CommonConfigurationKeys.LANGUAGE_VERSION_SETTINGS,
@@ -56,7 +56,8 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
if (descriptor !is MemberDescriptor) return if (descriptor !is MemberDescriptor) return
if (descriptor.isPlatform && declaration.hasModifier(KtTokens.PLATFORM_KEYWORD)) { if (descriptor.isPlatform && declaration.hasModifier(KtTokens.PLATFORM_KEYWORD)) {
checkPlatformDeclarationHasDefinition(declaration, descriptor, diagnosticHolder) val checkImpl = !languageVersionSettings.supportsFeature(LanguageFeature.MultiPlatformDoNotCheckImpl)
checkPlatformDeclarationHasDefinition(declaration, descriptor, diagnosticHolder, checkImpl)
} }
else if (descriptor.isImpl && declaration.hasModifier(KtTokens.IMPL_KEYWORD)) { else if (descriptor.isImpl && declaration.hasModifier(KtTokens.IMPL_KEYWORD)) {
checkImplementationHasPlatformDeclaration(declaration, descriptor, diagnosticHolder) checkImplementationHasPlatformDeclaration(declaration, descriptor, diagnosticHolder)
@@ -64,7 +65,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
} }
private fun checkPlatformDeclarationHasDefinition( private fun checkPlatformDeclarationHasDefinition(
reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink, checkImpl: Boolean
) { ) {
val compatibility = when (descriptor) { val compatibility = when (descriptor) {
is CallableMemberDescriptor -> { is CallableMemberDescriptor -> {
@@ -73,7 +74,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
// TODO: support non-source definitions (e.g. from Java) // TODO: support non-source definitions (e.g. from Java)
DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement
}.groupBy { impl -> }.groupBy { impl ->
areCompatibleCallables(descriptor, impl) areCompatibleCallables(descriptor, impl, checkImpl)
} }
} }
is ClassDescriptor -> { is ClassDescriptor -> {
@@ -81,7 +82,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
descriptor != impl && descriptor != impl &&
DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement
}.groupBy { impl -> }.groupBy { impl ->
areCompatibleClassifiers(descriptor, impl) areCompatibleClassifiers(descriptor, impl, checkImpl)
} }
} }
else -> null else -> null
@@ -102,7 +103,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
findClassifiersFromTheSameModule().firstOrNull { declaration -> findClassifiersFromTheSameModule().firstOrNull { declaration ->
this != declaration && this != declaration &&
declaration is ClassDescriptor && declaration.isPlatform && declaration is ClassDescriptor && declaration.isPlatform &&
areCompatibleClassifiers(declaration, this) == Compatible areCompatibleClassifiers(declaration, this, checkImpl = false) == Compatible
} as? ClassDescriptor } as? ClassDescriptor
val hasDeclaration = when (descriptor) { val hasDeclaration = when (descriptor) {
@@ -116,7 +117,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
candidates.any { declaration -> candidates.any { declaration ->
descriptor != declaration && descriptor != declaration &&
declaration.isPlatform && declaration.isPlatform &&
areCompatibleCallables(declaration, descriptor) == Compatible areCompatibleCallables(declaration, descriptor, checkImpl = false) == Compatible
} }
} }
is ClassifierDescriptor -> descriptor.findDeclarationForClass() != null is ClassifierDescriptor -> descriptor.findDeclarationForClass() != null
@@ -202,6 +203,8 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
object TypeParameterVariance : Incompatible("declaration-site variances of type parameters are different") object TypeParameterVariance : Incompatible("declaration-site variances of type parameters are different")
object TypeParameterReified : Incompatible("some type parameter is reified in one declaration and non-reified in the other") object TypeParameterReified : Incompatible("some type parameter is reified in one declaration and non-reified in the other")
object NoImpl : Incompatible("the implementation is not marked with the 'impl' modifier (-Xno-check-impl)")
object Unknown : Incompatible(null) object Unknown : Incompatible(null)
} }
@@ -209,7 +212,12 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
} }
// a is the declaration in common code, b is the definition in the platform-specific code // a is the declaration in common code, b is the definition in the platform-specific code
private fun areCompatibleCallables(a: CallableMemberDescriptor, b: CallableMemberDescriptor, parentSubstitutor: Substitutor? = null): Compatibility { private fun areCompatibleCallables(
a: CallableMemberDescriptor,
b: CallableMemberDescriptor,
checkImpl: Boolean,
parentSubstitutor: Substitutor? = null
): Compatibility {
assert(a.name == b.name) { "This function should be invoked only for declarations with the same name: $a, $b" } assert(a.name == b.name) { "This function should be invoked only for declarations with the same name: $a, $b" }
assert(a.containingDeclaration is ClassDescriptor == b.containingDeclaration is ClassDescriptor) { assert(a.containingDeclaration is ClassDescriptor == b.containingDeclaration is ClassDescriptor) {
"This function should be invoked only for declarations in the same kind of container (both members or both top level): $a, $b" "This function should be invoked only for declarations in the same kind of container (both members or both top level): $a, $b"
@@ -250,7 +258,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
else -> throw AssertionError("Unsupported declarations: $a, $b") else -> throw AssertionError("Unsupported declarations: $a, $b")
} }
// TODO: check 'impl' modifier if (checkImpl && !b.isImpl) return Incompatible.NoImpl
return Compatible return Compatible
} }
@@ -278,15 +286,17 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
return Compatible return Compatible
} }
private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor): Compatibility { private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor, checkImpl: Boolean): Compatibility {
assert(a.fqNameUnsafe == other.fqNameUnsafe) { "This function should be invoked only for declarations with the same name: $a, $other" } assert(a.fqNameUnsafe == other.fqNameUnsafe) { "This function should be invoked only for declarations with the same name: $a, $other" }
var parentSubstitutor: Substitutor? = null var parentSubstitutor: Substitutor? = null
var implTypealias = false
val b = when (other) { val b = when (other) {
is ClassDescriptor -> other is ClassDescriptor -> other
is TypeAliasDescriptor -> { is TypeAliasDescriptor -> {
val classDescriptor = other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias val classDescriptor = other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias
implTypealias = true
// If a platform class test.C is implemented by a typealias test.C = test.CImpl, we must now state that any occurrence // If a platform class test.C is implemented by a typealias test.C = test.CImpl, we must now state that any occurrence
// of the type "test.C" in the platform class scope should be replaced with "test.CImpl". // of the type "test.C" in the platform class scope should be replaced with "test.CImpl".
// Otherwise the types would not be equal and e.g. test.C's constructor is not going to be found in test.CImpl's scope. // Otherwise the types would not be equal and e.g. test.C's constructor is not going to be found in test.CImpl's scope.
@@ -317,20 +327,29 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
if (!b.typeConstructor.supertypes.containsAll(a.typeConstructor.supertypes.map(substitutor))) return Incompatible.Supertypes if (!b.typeConstructor.supertypes.containsAll(a.typeConstructor.supertypes.map(substitutor))) return Incompatible.Supertypes
areCompatibleClassScopes(a, b, substitutor).let { if (it != Compatible) return it } areCompatibleClassScopes(a, b, checkImpl && !implTypealias, substitutor).let { if (it != Compatible) return it }
// TODO: check 'impl' modifier if (checkImpl && !b.isImpl && !implTypealias) return Incompatible.NoImpl
return Compatible return Compatible
} }
private fun areCompatibleClassScopes(a: ClassDescriptor, b: ClassDescriptor, substitutor: Substitutor): Compatibility { private fun areCompatibleClassScopes(
a: ClassDescriptor,
b: ClassDescriptor,
checkImpl: Boolean,
substitutor: Substitutor
): Compatibility {
val unimplemented = arrayListOf<Pair<CallableMemberDescriptor, Map<Incompatible, MutableCollection<CallableMemberDescriptor>>>>() val unimplemented = arrayListOf<Pair<CallableMemberDescriptor, Map<Incompatible, MutableCollection<CallableMemberDescriptor>>>>()
val bMembersByName = b.getMembers().groupBy { it.name } val bMembersByName = b.getMembers().groupBy { it.name }
outer@ for (aMember in a.getMembers()) { outer@ for (aMember in a.getMembers()) {
val mapping = bMembersByName[aMember.name].orEmpty().keysToMap { bMember -> areCompatibleCallables(aMember, bMember, substitutor) } if (!aMember.kind.isReal) continue
val mapping = bMembersByName[aMember.name].orEmpty().keysToMap { bMember ->
areCompatibleCallables(aMember, bMember, checkImpl, substitutor)
}
if (mapping.values.any { it == Compatible }) continue if (mapping.values.any { it == Compatible }) continue
val incompatibilityMap = mutableMapOf<Incompatible, MutableCollection<CallableMemberDescriptor>>() val incompatibilityMap = mutableMapOf<Incompatible, MutableCollection<CallableMemberDescriptor>>()
+1
View File
@@ -4,6 +4,7 @@ where advanced options include:
-Xrepeat <count> Repeat compilation (for performance analysis) -Xrepeat <count> Repeat compilation (for performance analysis)
-Xplugin <path> Load plugins from the given classpath -Xplugin <path> Load plugins from the given classpath
-Xmulti-platform Enable experimental language support for multi-platform projects -Xmulti-platform Enable experimental language support for multi-platform projects
-Xno-check-impl Do not check presence of 'impl' modifier in multi-platform projects
Advanced options are non-standard and may be changed or removed without any notice. Advanced options are non-standard and may be changed or removed without any notice.
OK OK
+1
View File
@@ -17,6 +17,7 @@ where advanced options include:
-Xrepeat <count> Repeat compilation (for performance analysis) -Xrepeat <count> Repeat compilation (for performance analysis)
-Xplugin <path> Load plugins from the given classpath -Xplugin <path> Load plugins from the given classpath
-Xmulti-platform Enable experimental language support for multi-platform projects -Xmulti-platform Enable experimental language support for multi-platform projects
-Xno-check-impl Do not check presence of 'impl' modifier in multi-platform projects
Advanced options are non-standard and may be changed or removed without any notice. Advanced options are non-standard and may be changed or removed without any notice.
OK OK
@@ -0,0 +1,7 @@
platform open class Base {
fun base()
}
platform class Derived : Base {
fun derived()
}
@@ -0,0 +1,7 @@
impl open class Base {
impl fun base() {}
}
impl class Derived : Base() {
impl fun derived() {}
}
@@ -0,0 +1,7 @@
-- Common --
Exit code: OK
Output:
-- JVM --
Exit code: OK
Output:
+1 -1
View File
@@ -1,5 +1,5 @@
impl class Printer { impl class Printer {
fun print(message: String) { impl fun print(message: String) {
println("JS says: " + message) println("JS says: " + message)
} }
} }
+1 -1
View File
@@ -1,5 +1,5 @@
impl class Printer { impl class Printer {
fun print(message: String) { impl fun print(message: String) {
println("JVM says: " + message) println("JVM says: " + message)
} }
} }
@@ -98,6 +98,12 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform
doTest(fileName); doTest(fileName);
} }
@TestMetadata("fakeOverrides")
public void testFakeOverrides() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/fakeOverrides/");
doTest(fileName);
}
@TestMetadata("functionIncorrectSignature") @TestMetadata("functionIncorrectSignature")
public void testFunctionIncorrectSignature() throws Exception { public void testFunctionIncorrectSignature() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/functionIncorrectSignature/"); String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/functionIncorrectSignature/");
@@ -36,6 +36,7 @@ enum class LanguageFeature(val sinceVersion: LanguageVersion?) {
// Experimental features // Experimental features
MultiPlatformProjects(null), MultiPlatformProjects(null),
MultiPlatformDoNotCheckImpl(null),
; ;
val presentableText: String val presentableText: String