From 475fb6e8a782e087136caff1b0d35bb9d40199ed Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 22 Jul 2016 13:31:15 +0300 Subject: [PATCH] Simplify OverrideResolver#filterOverrides - move source vs binary equivalent call filtering hack to ResolutionResultsHandler (see c4778bfe5a4fe97826eae6ade149be95b30ad2c2 for the original commit introducing this behavior) - rewrite the algorithm so that it doesn't need noDuplicates in the beginning: modulo the hack above whish is only needed in ResolutionResultsHandler, descriptor equivalence is checked anyway in OverrideResolver#overrides --- .../kotlin/resolve/OverrideResolver.java | 67 ++++--------------- .../results/ResolutionResultsHandler.java | 48 +++++++++++-- .../jetbrains/kotlin/utils/HashSetUtil.java | 14 ---- 3 files changed, 57 insertions(+), 72 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java index ba835793ca2..94ef2e84170 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java @@ -22,7 +22,6 @@ import com.intellij.psi.PsiElement; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.SmartHashSet; -import com.intellij.util.containers.hash.EqualityPolicy; import kotlin.Unit; import kotlin.collections.CollectionsKt; import kotlin.jvm.functions.Function1; @@ -40,11 +39,9 @@ import org.jetbrains.kotlin.resolve.dataClassUtils.DataClassUtilsKt; import org.jetbrains.kotlin.types.*; import org.jetbrains.kotlin.types.checker.KotlinTypeChecker; import org.jetbrains.kotlin.utils.FunctionsKt; -import org.jetbrains.kotlin.utils.HashSetUtil; import java.util.*; -import static kotlin.collections.CollectionsKt.sortedBy; import static org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.DELEGATION; import static org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.FAKE_OVERRIDE; import static org.jetbrains.kotlin.diagnostics.Errors.*; @@ -108,45 +105,6 @@ public class OverrideResolver { return filterOverrides(candidateSet, FunctionsKt.identity()); } - // In a multi-module project different "copies" of the same class may be present in different libraries, - // that's why we use structural equivalence for members (DescriptorEquivalenceForOverrides). - // - // Sometimes we should compare "copies" from sources and from binary files. - // But we cannot compare return types for such copies, because it may lead us to recursive problem (see KT-11995). - // Because of this we compare them without return type and choose descriptor from source if we found duplicate. - @NotNull - private static Set noDuplicates( - @NotNull Set candidateSet, - @NotNull final Function1 transform - ) { - List fromSourcesGoesFirst = sortedBy(candidateSet, new Function1() { - @Override - public Integer invoke(D d) { - return DescriptorToSourceUtils.descriptorToDeclaration(transform.invoke(d)) != null ? 0 : 1; - } - }); - - return HashSetUtil.linkedHashSet( - fromSourcesGoesFirst, - new EqualityPolicy() { - @Override - public int getHashCode(D d) { - return DescriptorUtils.getFqName(transform.invoke(d).getContainingDeclaration()).hashCode(); - } - - @Override - public boolean isEqual(D d1, D d2) { - CallableDescriptor f = transform.invoke(d1).getOriginal(); - CallableDescriptor g = transform.invoke(d2).getOriginal(); - - boolean ignoreReturnType = (DescriptorToSourceUtils.descriptorToDeclaration(f) == null) != - (DescriptorToSourceUtils.descriptorToDeclaration(g) == null); - - return DescriptorEquivalenceForOverrides.INSTANCE.areCallableDescriptorsEquivalent(f, g, ignoreReturnType); - } - }); - } - @NotNull public static Set filterOverrides( @NotNull Set candidateSet, @@ -154,33 +112,34 @@ public class OverrideResolver { ) { if (candidateSet.size() <= 1) return candidateSet; - // Here we filter out structurally equivalent descriptors before processing overrides, because such descriptors - // "override" each other (overrides(f, g) = overrides(g, f) = true) and the code below removes them all from the - // candidates, unless we first compute noDuplicates - Set noDuplicates = noDuplicates(candidateSet, transform); - - Set candidates = Sets.newLinkedHashSet(); + Set result = new LinkedHashSet(); outerLoop: - for (D meD : noDuplicates) { + for (D meD : candidateSet) { CallableDescriptor me = transform.invoke(meD); - for (D otherD : noDuplicates) { + for (Iterator iterator = result.iterator(); iterator.hasNext(); ) { + D otherD = iterator.next(); CallableDescriptor other = transform.invoke(otherD); - if (me != other && overrides(other, me)) { + if (overrides(me, other)) { + iterator.remove(); + } + else if (overrides(other, me)) { continue outerLoop; } } - candidates.add(meD); + result.add(meD); } - assert !candidates.isEmpty() : "All candidates filtered out from " + candidateSet; + assert !result.isEmpty() : "All candidates filtered out from " + candidateSet; - return candidates; + return result; } /** * @return whether f overrides g */ public static boolean overrides(@NotNull D f, @NotNull D g) { + // In a multi-module project different "copies" of the same class may be present in different libraries, + // that's why we use structural equivalence for members (DescriptorEquivalenceForOverrides). // This first check cover the case of duplicate classes in different modules: // when B is defined in modules m1 and m2, and C (indirectly) inherits from both versions, // we'll be getting sets of members that do not override each other, but are structurally equivalent. diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/results/ResolutionResultsHandler.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/results/ResolutionResultsHandler.java index c0dc08508ba..edeb21df04c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/results/ResolutionResultsHandler.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/results/ResolutionResultsHandler.java @@ -17,10 +17,13 @@ package org.jetbrains.kotlin.resolve.calls.results; import com.google.common.collect.Sets; +import kotlin.collections.CollectionsKt; import kotlin.jvm.functions.Function1; import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.descriptors.CallableDescriptor; import org.jetbrains.kotlin.resolve.BindingTrace; +import org.jetbrains.kotlin.resolve.DescriptorEquivalenceForOverrides; +import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; import org.jetbrains.kotlin.resolve.OverrideResolver; import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.context.CallResolutionContext; @@ -29,9 +32,7 @@ import org.jetbrains.kotlin.resolve.calls.model.MutableResolvedCall; import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall; import org.jetbrains.kotlin.resolve.calls.tasks.TracingStrategy; -import java.util.Collection; -import java.util.EnumSet; -import java.util.Set; +import java.util.*; import static org.jetbrains.kotlin.resolve.calls.results.ResolutionStatus.*; @@ -44,6 +45,14 @@ public class ResolutionResultsHandler { } }; + private static final Function1, Integer> MAP_RESOLVED_CALL_TO_SOURCE_PRESENCE = + new Function1, Integer>() { + @Override + public Integer invoke(MutableResolvedCall resolvedCall) { + return DescriptorToSourceUtils.descriptorToDeclaration(resolvedCall.getResultingDescriptor()) != null ? 0 : 1; + } + }; + private final OverloadingConflictResolver overloadingConflictResolver; public ResolutionResultsHandler(@NotNull OverloadingConflictResolver overloadingConflictResolver) { @@ -186,6 +195,35 @@ public class ResolutionResultsHandler { return true; } + // Sometimes we should compare "copies" from sources and from binary files. + // But we cannot compare return types for such copies, because it may lead us to recursive problem (see KT-11995). + // Because of this we compare them without return type and choose descriptor from source if we found duplicate. + @NotNull + private static Set> filterOutEquivalentCalls( + @NotNull Set> candidates + ) { + if (candidates.size() <= 1) return candidates; + + List> fromSourcesGoesFirst = CollectionsKt.sortedBy(candidates, MAP_RESOLVED_CALL_TO_SOURCE_PRESENCE); + + Set> result = new LinkedHashSet>(); + outerLoop: + for (MutableResolvedCall meD : fromSourcesGoesFirst) { + for (MutableResolvedCall otherD : result) { + D me = meD.getResultingDescriptor(); + D other = otherD.getResultingDescriptor(); + boolean ignoreReturnType = (DescriptorToSourceUtils.descriptorToDeclaration(me) == null) != + (DescriptorToSourceUtils.descriptorToDeclaration(other) == null); + if (DescriptorEquivalenceForOverrides.INSTANCE.areCallableDescriptorsEquivalent(me, other, ignoreReturnType)) { + continue outerLoop; + } + } + result.add(meD); + } + + return result; + } + @NotNull private OverloadResolutionResultsImpl chooseAndReportMaximallySpecific( @NotNull Set> candidates, @@ -201,7 +239,9 @@ public class ResolutionResultsHandler { candidates = overloadingConflictResolver.findMaximallySpecificVariableAsFunctionCalls(candidates); } - Set> noOverrides = OverrideResolver.filterOverrides(candidates, MAP_RESOLVED_CALL_TO_RESULTING_DESCRIPTOR); + Set> noEquivalentCalls = filterOutEquivalentCalls(candidates); + Set> noOverrides = + OverrideResolver.filterOverrides(noEquivalentCalls, MAP_RESOLVED_CALL_TO_RESULTING_DESCRIPTOR); if (noOverrides.size() == 1) { return OverloadResolutionResultsImpl.success(noOverrides.iterator().next()); } diff --git a/compiler/util/src/org/jetbrains/kotlin/utils/HashSetUtil.java b/compiler/util/src/org/jetbrains/kotlin/utils/HashSetUtil.java index fdd6e283aa1..ff73f32499b 100644 --- a/compiler/util/src/org/jetbrains/kotlin/utils/HashSetUtil.java +++ b/compiler/util/src/org/jetbrains/kotlin/utils/HashSetUtil.java @@ -16,26 +16,12 @@ package org.jetbrains.kotlin.utils; -import com.intellij.util.containers.hash.EqualityPolicy; import com.intellij.util.containers.hash.HashSet; -import com.intellij.util.containers.hash.LinkedHashMap; import org.jetbrains.annotations.NotNull; -import java.util.Collection; -import java.util.Map; import java.util.Set; public class HashSetUtil { - @NotNull - public static Set linkedHashSet(@NotNull Collection set, @NotNull EqualityPolicy policy) { - // this implementation of LinkedHashMap doesn't admit nulls as values - Map map = new LinkedHashMap(policy); - for (T t : set) { - map.put(t, ""); - } - return map.keySet(); - } - @NotNull public static Set symmetricDifference(@NotNull Set set1, @NotNull Set set2) { Set intersection = new HashSet();