From c5373c9029fe188ad6d49189e34c82d8a6585fb1 Mon Sep 17 00:00:00 2001 From: Denis Vnukov Date: Fri, 20 Apr 2018 02:27:30 -0700 Subject: [PATCH] Ensure deterministic insertion of checkExpressionValueIsNotNull. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kotlinc source’s file DescriptorUtils.kt demonstarted non-deterministic insertion of checkExpressionValueIsNotNull for value returned by CallableDescriptor::getOriginal(). It was difficult to reproduce this behavior on ф smaller example, but I added a test which was failing once in 5-10 times while I was testing manually. I believe this bug is close to KT-23704. This PR addresses non-determinism to a degree when I can run 120 Compilations with './gradlew dist' and get same classes in all jars in 'dist'. NOTE that thew fact that insertion of checkExpressionValueIsNotNull may depend on order of the types seems suspicios. This CL only addresses non-determinism part, but I believe it’s worth looking into this more from semantics point of view. --- .../kotlin/types/CommonSupertypes.java | 10 +- .../deterministicNotNullChecks.kt | 109 ++++++++++++++++++ .../codegen/BytecodeTextTestGenerated.java | 6 + 3 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/CommonSupertypes.java b/compiler/frontend/src/org/jetbrains/kotlin/types/CommonSupertypes.java index e976b169711..f26c0ae4179 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/CommonSupertypes.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/CommonSupertypes.java @@ -100,7 +100,7 @@ public class CommonSupertypes { @NotNull private static SimpleType commonSuperTypeForInflexible(@NotNull Collection types, int recursionDepth, int maxDepth) { assert !types.isEmpty(); - Collection typeSet = new HashSet<>(types); + Collection typeSet = new LinkedHashSet<>(types); // If any of the types is nullable, the result must be nullable // This also removed Nothing and Nothing? because they are subtypes of everything else @@ -132,7 +132,7 @@ public class CommonSupertypes { // constructor of the supertype -> all of its instantiations occurring as supertypes Map> commonSupertypes = computeCommonRawSupertypes(typeSet); while (commonSupertypes.size() > 1) { - Set merge = new HashSet<>(); + Set merge = new LinkedHashSet<>(); for (Set supertypes : commonSupertypes.values()) { merge.addAll(supertypes); } @@ -244,7 +244,7 @@ public class CommonSupertypes { List parameters = constructor.getParameters(); List newProjections = new ArrayList<>(parameters.size()); for (TypeParameterDescriptor parameterDescriptor : parameters) { - Set typeProjections = new HashSet<>(); + Set typeProjections = new LinkedHashSet<>(); for (KotlinType type : types) { typeProjections.add(type.getArguments().get(parameterDescriptor.getIndex())); } @@ -287,8 +287,8 @@ public class CommonSupertypes { return TypeUtils.makeStarProjection(parameterDescriptor); } - Set ins = new HashSet<>(); - Set outs = new HashSet<>(); + Set ins = new LinkedHashSet<>(); + Set outs = new LinkedHashSet<>(); Variance variance = parameterDescriptor.getVariance(); switch (variance) { diff --git a/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt new file mode 100644 index 00000000000..3f3edaca1e8 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt @@ -0,0 +1,109 @@ +// FILE: test/DeclarationDescriptor.java + +package test; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +public interface DeclarationDescriptor { + @NotNull + DeclarationDescriptor getOriginal(); + + @Nullable + DeclarationDescriptor getContainingDeclaration(); +} + +// FILE: test/DeclarationDescriptorWithVisibility.java +package test; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +public interface DeclarationDescriptorWithVisibility extends DeclarationDescriptor { +} + +// FILE: test/DeclarationDescriptorWithSource.java +package test; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +public interface DeclarationDescriptorWithSource extends DeclarationDescriptor { + @Override + @NotNull + DeclarationDescriptorWithSource getOriginal(); +} + +// FILE: test/DeclarationDescriptorNonRoot.java +package test; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +public interface DeclarationDescriptorNonRoot extends DeclarationDescriptorWithSource { +} + +// FILE: test/CallableDescriptor.java +package test; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +public interface CallableDescriptor extends DeclarationDescriptorWithVisibility, DeclarationDescriptorNonRoot { + @NotNull + @Override + CallableDescriptor getOriginal(); + + @NotNull + Collection getOverriddenDescriptors(); +} + +// FILE: test/k.kt +package test + +fun D.overriddenTreeUniqueAsSequenceA(useOriginal: Boolean): Sequence { + val set = hashSetOf() + + @Suppress("UNCHECKED_CAST") + fun D.doBuildOverriddenTreeAsSequence(): Sequence { + return with(if (useOriginal) original as D else this) { + if (original in set) + emptySequence() + else { + emptySequence() + } + } + } + + return doBuildOverriddenTreeAsSequence() +} + +fun D.overriddenTreeUniqueAsSequenceB(useOriginal: Boolean): Sequence { + val set = hashSetOf() + + @Suppress("UNCHECKED_CAST") + fun D.doBuildOverriddenTreeAsSequence(): Sequence { + return with(if (useOriginal) original as D else this) { + if (original in set) + emptySequence() + else { + emptySequence() + } + } + } + + return doBuildOverriddenTreeAsSequence() +} + +// @KKt.class: +// 0 checkExpressionValueIsNotNull diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index d58126457d2..38959510ab5 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -2344,6 +2344,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/alreadyCheckedForNull.kt"); } + @TestMetadata("deterministicNotNullChecks.kt") + public void testDeterministicNotNullChecks() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/nullCheckOptimization/deterministicNotNullChecks.kt"); + doTest(fileName); + } + @TestMetadata("expressionValueIsNotNull.kt") public void testExpressionValueIsNotNull() throws Exception { runTest("compiler/testData/codegen/bytecodeText/nullCheckOptimization/expressionValueIsNotNull.kt");