From 8ee1096e374061fdf85f3de4461bbe1dfed33be7 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Tue, 13 Sep 2011 20:27:44 +0400 Subject: [PATCH] KT-286 Check supertype lists (in progress) Consistency for type arguments in the supertype list --- .../jet/lang/psi/JetNullableType.java | 6 ++ .../lang/resolve/TypeHierarchyResolver.java | 101 ++++++++++++++++-- .../jetbrains/jet/lang/types/TypeUtils.java | 26 +++-- .../jetbrains/jet/util/CommonSuppliers.java | 14 +++ .../checker/GenericArgumentConsistency.jet | 42 ++++++++ .../checker/ProjectionsInSupertypes.jet | 2 +- 6 files changed, 177 insertions(+), 14 deletions(-) create mode 100644 idea/testData/checker/GenericArgumentConsistency.jet diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetNullableType.java b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetNullableType.java index f674154a740..0e1e049da29 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetNullableType.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetNullableType.java @@ -2,6 +2,7 @@ package org.jetbrains.jet.lang.psi; import com.intellij.lang.ASTNode; import org.jetbrains.annotations.NotNull; +import org.jetbrains.jet.lexer.JetTokens; import java.util.List; @@ -12,6 +13,11 @@ public class JetNullableType extends JetTypeElement { public JetNullableType(@NotNull ASTNode node) { super(node); } + + @NotNull + public ASTNode getQuestionMarkNode() { + return getNode().findChildByType(JetTokens.QUEST); + } @NotNull @Override diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java index 700f8522c75..b92d0ea64ed 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeHierarchyResolver.java @@ -1,6 +1,7 @@ package org.jetbrains.jet.lang.resolve; import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.intellij.lang.ASTNode; import com.intellij.psi.PsiElement; @@ -10,8 +11,7 @@ import org.jetbrains.jet.lang.cfg.JetFlowInformationProvider; import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.jet.lang.psi.*; -import org.jetbrains.jet.lang.types.JetType; -import org.jetbrains.jet.lang.types.JetTypeInferrer; +import org.jetbrains.jet.lang.types.*; import org.jetbrains.jet.lexer.JetTokens; import java.util.*; @@ -24,6 +24,8 @@ import static org.jetbrains.jet.lang.resolve.BindingContext.TYPE; */ public class TypeHierarchyResolver { private final TopDownAnalysisContext context; + private LinkedList topologicalOrder; + public TypeHierarchyResolver(TopDownAnalysisContext context) { this.context = context; @@ -35,14 +37,20 @@ public class TypeHierarchyResolver { createTypeConstructors(); // create type constructors for classes and generic parameters, supertypes are not filled in resolveTypesInClassHeaders(); // Generic bounds and types in supertype lists (no expressions or constructor resolution) + topologicalOrder = topologicallySortClassesAndObjects(); + // Detect and disconnect all loops in the hierarchy detectAndDisconnectLoops(); + // At this point, there are no loops in the type hierarchy + + checkSupertypesForConsistency(); +// computeSuperclasses(); + // Add supertypes to resolution scopes of classes addSupertypesToScopes(); - - checkTypesInClassHeaders(); // Generic bounds and supertype lists + checkTypesInClassHeaders(); // Check bounds in the types used in generic bounds and supertype lists } private void collectNamespacesAndClassifiers( @@ -269,7 +277,7 @@ public class TypeHierarchyResolver { } } - private void detectAndDisconnectLoops() { + private LinkedList topologicallySortClassesAndObjects() { // A topsort is needed only for better diagnostics: // edges that get removed to disconnect loops are more reasonable in this case LinkedList topologicalOrder = Lists.newLinkedList(); @@ -277,9 +285,15 @@ public class TypeHierarchyResolver { for (MutableClassDescriptor mutableClassDescriptor : context.getClasses().values()) { topologicallySort(mutableClassDescriptor, visited, topologicalOrder); } + for (MutableClassDescriptor mutableClassDescriptor : context.getObjects().values()) { + topologicallySort(mutableClassDescriptor, visited, topologicalOrder); + } + return topologicalOrder; + } + private void detectAndDisconnectLoops() { // Loop detection and disconnection - visited.clear(); + Set visited = Sets.newHashSet(); Set beingProcessed = Sets.newHashSet(); List currentPath = Lists.newArrayList(); for (MutableClassDescriptor mutableClassDescriptor : topologicalOrder) { @@ -369,6 +383,81 @@ public class TypeHierarchyResolver { } } + private void checkSupertypesForConsistency() { + for (MutableClassDescriptor mutableClassDescriptor : topologicalOrder) { + Multimap multimap = TypeUtils.buildDeepSubstitutionMultimap(mutableClassDescriptor.getDefaultType()); + for (Map.Entry> entry : multimap.asMap().entrySet()) { + Collection projections = entry.getValue(); + if (projections.size() > 1) { + TypeConstructor typeConstructor = entry.getKey(); + DeclarationDescriptor declarationDescriptor = typeConstructor.getDeclarationDescriptor(); + assert declarationDescriptor instanceof TypeParameterDescriptor : declarationDescriptor; + TypeParameterDescriptor typeParameterDescriptor = (TypeParameterDescriptor) declarationDescriptor; + + // Immediate arguments of supertypes cannot be projected + Set conflictingTypes = Sets.newLinkedHashSet(); + for (TypeProjection projection : projections) { + conflictingTypes.add(projection.getType()); + } + switch (typeParameterDescriptor.getVariance()) { + case INVARIANT: + // Leave conflicting types as is + break; + case IN_VARIANCE: + // Filter out those who have supertypes in this set (common supertype) + Filter.REMOVE_IF_SUPERTYPE_IN_THE_SET.proceed(conflictingTypes); + break; + case OUT_VARIANCE: + // Filter out those who have subtypes in this set (common subtype) + Filter.REMOVE_IF_SUBTYPE_IN_THE_SET.proceed(conflictingTypes); + break; + } + + if (conflictingTypes.size() > 1) { + DeclarationDescriptor containingDeclaration = typeParameterDescriptor.getContainingDeclaration(); + assert containingDeclaration instanceof ClassDescriptor : containingDeclaration; + PsiElement psiElement = context.getTrace().get(DESCRIPTOR_TO_DECLARATION, mutableClassDescriptor); + assert psiElement instanceof JetClassOrObject : psiElement; + JetClassOrObject declaration = (JetClassOrObject) psiElement; + JetDelegationSpecifierList delegationSpecifierList = declaration.getDelegationSpecifierList(); + assert delegationSpecifierList != null; + context.getTrace().getErrorHandler().genericError(delegationSpecifierList.getNode(), "Type parameter " + typeParameterDescriptor.getName() + " of " + containingDeclaration.getName() + " has inconsistent values: " + conflictingTypes); + } + } + } + } + } + + private enum Filter { + REMOVE_IF_SUBTYPE_IN_THE_SET { + @Override + public boolean removeNeeded(JetType subject, JetType other) { + return JetTypeChecker.INSTANCE.isSubtypeOf(other, subject); + } + }, + REMOVE_IF_SUPERTYPE_IN_THE_SET { + @Override + public boolean removeNeeded(JetType subject, JetType other) { + return JetTypeChecker.INSTANCE.isSubtypeOf(subject, other); + } + }; + + private void proceed(Set conflictingTypes) { + for (Iterator iterator = conflictingTypes.iterator(); iterator.hasNext(); ) { + JetType type = iterator.next(); + for (JetType otherType : conflictingTypes) { + boolean subtypeOf = removeNeeded(type, otherType); + if (type != otherType && subtypeOf) { + iterator.remove(); + break; + } + } + } + } + + public abstract boolean removeNeeded(JetType subject, JetType other); + } + private void addSupertypesToScopes() { for (MutableClassDescriptor mutableClassDescriptor : context.getClasses().values()) { mutableClassDescriptor.addSupertypesToScopeForMemberLookup(); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeUtils.java index 9a675d51f94..9d5a28a5844 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/TypeUtils.java @@ -1,8 +1,6 @@ package org.jetbrains.jet.lang.types; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; +import com.google.common.collect.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.ClassDescriptor; @@ -11,6 +9,7 @@ import org.jetbrains.jet.lang.descriptors.TypeParameterDescriptor; import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.jet.lang.resolve.ChainedScope; import org.jetbrains.jet.lang.resolve.JetScope; +import org.jetbrains.jet.util.CommonSuppliers; import java.util.*; @@ -229,15 +228,25 @@ public class TypeUtils { */ @NotNull public static TypeSubstitutor buildDeepSubstitutor(@NotNull JetType type) { - HashMap substitution = Maps.newHashMap(); + Map substitution = Maps.newHashMap(); TypeSubstitutor typeSubstitutor = TypeSubstitutor.create(substitution); // we use the mutability of the map here - fillInDeepSubstitutor(type, typeSubstitutor, substitution); + fillInDeepSubstitutor(type, typeSubstitutor, substitution, null); return typeSubstitutor; } + @NotNull + public static Multimap buildDeepSubstitutionMultimap(@NotNull JetType type) { + Multimap fullSubstitution = Multimaps.newSetMultimap(Maps.>newHashMap(), CommonSuppliers.getLinkedHashSetSupplier()); + Map substitution = Maps.newHashMap(); + TypeSubstitutor typeSubstitutor = TypeSubstitutor.create(substitution); + // we use the mutability of the map here + fillInDeepSubstitutor(type, typeSubstitutor, substitution, fullSubstitution); + return fullSubstitution; + } + // we use the mutability of the substitution map here - private static void fillInDeepSubstitutor(JetType context, TypeSubstitutor substitutor, Map substitution) { + private static void fillInDeepSubstitutor(@NotNull JetType context, @NotNull TypeSubstitutor substitutor, @NotNull Map substitution, @Nullable Multimap fullSubstitution) { List parameters = context.getConstructor().getParameters(); List arguments = context.getArguments(); for (int i = 0; i < arguments.size(); i++) { @@ -248,9 +257,12 @@ public class TypeUtils { assert substitute != null; TypeProjection substitutedTypeProjection = new TypeProjection(argument.getProjectionKind(), substitute); substitution.put(typeParameterDescriptor.getTypeConstructor(), substitutedTypeProjection); + if (fullSubstitution != null) { + fullSubstitution.put(typeParameterDescriptor.getTypeConstructor(), substitutedTypeProjection); + } } for (JetType supertype : context.getConstructor().getSupertypes()) { - fillInDeepSubstitutor(supertype, substitutor, substitution); + fillInDeepSubstitutor(supertype, substitutor, substitution, fullSubstitution); } } diff --git a/compiler/frontend/src/org/jetbrains/jet/util/CommonSuppliers.java b/compiler/frontend/src/org/jetbrains/jet/util/CommonSuppliers.java index a4185670309..b596e476a44 100644 --- a/compiler/frontend/src/org/jetbrains/jet/util/CommonSuppliers.java +++ b/compiler/frontend/src/org/jetbrains/jet/util/CommonSuppliers.java @@ -2,8 +2,10 @@ package org.jetbrains.jet.util; import com.google.common.base.Supplier; import com.google.common.collect.Lists; +import com.google.common.collect.Sets; import java.util.List; +import java.util.Set; /** * @author abreslav @@ -18,8 +20,20 @@ public class CommonSuppliers { } }; + private static final Supplier LINKED_HASH_SET_SUPPLIER = new Supplier() { + @Override + public Set get() { + return Sets.newLinkedHashSet(); + } + }; + public static Supplier> getArrayListSupplier() { //noinspection unchecked return (Supplier>) ARRAY_LIST_SUPPLIER; } + + public static Supplier> getLinkedHashSetSupplier() { + //noinspection unchecked + return (Supplier>) LINKED_HASH_SET_SUPPLIER; + } } diff --git a/idea/testData/checker/GenericArgumentConsistency.jet b/idea/testData/checker/GenericArgumentConsistency.jet new file mode 100644 index 00000000000..658b84dc9d4 --- /dev/null +++ b/idea/testData/checker/GenericArgumentConsistency.jet @@ -0,0 +1,42 @@ +trait A {} +trait B : A {} +trait C : B, A {} +trait C1 : B, A {} +trait D : C, B{} + +trait A1 {} +trait B1 : A1 {} +trait B2 : A1, B1 {} + +trait BA1 {} +trait BB1 : BA1 {} +trait BB2 : BA1, BB1 {} + + +namespace x { + trait AA1 {} + trait AB1 : AA1 {} + trait AB3 : AA1> {} + trait AB2 : AA1, AB1, AB3 {} +} + +namespace x2 { + trait AA1 {} + trait AB1 : AA1 {} + trait AB3 : AA1> {} + trait AB2 : AA1, AB1, AB3 {} +} + +namespace x3 { + trait AA1 {} + trait AB1 : AA1 {} + trait AB3 : AA1> {} + trait AB2 : AA1, AB1, AB3 {} +} + +namespace sx2 { + trait AA1 {} + trait AB1 : AA1 {} + trait AB3 : AA1> {} + trait AB2 : AA1, AB1, AB3 {} +} \ No newline at end of file diff --git a/idea/testData/checker/ProjectionsInSupertypes.jet b/idea/testData/checker/ProjectionsInSupertypes.jet index c761aecec97..efddb978e07 100644 --- a/idea/testData/checker/ProjectionsInSupertypes.jet +++ b/idea/testData/checker/ProjectionsInSupertypes.jet @@ -3,4 +3,4 @@ trait B {} trait C {} trait D {} -trait Test : A<in Int>, B<out T>, C<*>???, D {} \ No newline at end of file +trait Test : A<in Int>, B<out Int>, C<*>???, D {} \ No newline at end of file