Generate bridges for trait implementations properly

The intent was to keep the bridge codegen model as simple as possible: we
should be able to figure out all necessary bridges only by a minimal interface
that FunctionHandle provides (isAbstract, isDeclaration, getOverridden)

Add different tests for bridges

 #KT-318 Obsolete

compiler/backend-common/src/bridges/impl.kt

@@ -16,8 +16,12 @@
 
 package org.jetbrains.jet.codegen.bridges
 
-import org.jetbrains.jet.lang.descriptors.FunctionDescriptor
-import org.jetbrains.jet.lang.descriptors.Modality
+import org.jetbrains.jet.lang.descriptors.*
+import org.jetbrains.jet.lang.resolve.DescriptorUtils
+import org.jetbrains.jet.lang.resolve.OverridingUtil
+import org.jetbrains.jet.lang.resolve.calls.CallResolverUtil
+import org.jetbrains.jet.lang.types.TypeUtils
+import java.util.LinkedHashSet
 
 public fun <Signature> generateBridgesForFunctionDescriptor(
         descriptor: FunctionDescriptor,
@@ -26,11 +30,72 @@ public fun <Signature> generateBridgesForFunctionDescriptor(
     return generateBridges(DescriptorBasedFunctionHandle(descriptor), { signature(it.descriptor) })
 }
 
+/**
+ * An implementation of FunctionHandle based on descriptors.
+ *
+ * This implementation workarounds a minor inconvenience in descriptor hierarchy regarding traits with implementations.
+ * Consider the following hierarchy:
+ *
+ * trait A { fun foo() = 42 }
+ * class B : A
+ *
+ * In terms of descriptors, we'll have a declaration in trait A with modality=OPEN and a fake override in class B with modality=OPEN.
+ * For the purposes of bridge generation though, it's much easier to "move" all implementations out of traits into their child classes,
+ * i.e. treat the function in A as a declaration with modality=ABSTRACT and a function in B as a _declaration_ with modality=OPEN.
+ *
+ * This provides us with the nice invariant that all implementations (concrete declarations) are always in classes. This means we _always_
+ * can generate a bridge near an implementation (of course, in case it has a super-declaration with a different signature). Ultimately this
+ * eases the process of determining what bridges are already generated in our supertypes and need to be inherited, not regenerated.
+ */
 private data class DescriptorBasedFunctionHandle(val descriptor: FunctionDescriptor) : FunctionHandle {
-    override val isDeclaration: Boolean = descriptor.getKind().isReal()
-    override val isAbstract: Boolean = descriptor.getModality() == Modality.ABSTRACT
+    private val _overridden = descriptor.getOverriddenDescriptors().map { DescriptorBasedFunctionHandle(it.getOriginal()) }
 
-    override fun getOverridden(): Iterable<FunctionHandle> {
-        return descriptor.getOverriddenDescriptors().map { DescriptorBasedFunctionHandle(it.getOriginal()) }
-    }
+    override val isDeclaration: Boolean =
+            descriptor.getKind().isReal() ||
+            findTraitImplementation(descriptor) != null
+
+    override val isAbstract: Boolean =
+            descriptor.getModality() == Modality.ABSTRACT ||
+            DescriptorUtils.isTrait(descriptor.getContainingDeclaration())
+
+    override fun getOverridden() = _overridden
+}
+
+
+/**
+ * Given a fake override in a class, returns an overridden declaration with implementation in trait, such that a method delegating to that
+ * trait implementation should be generated into the class containing the fake override; or null if the given function is not a fake
+ * override of any trait implementation or such method was already generated into some superclass
+ */
+public fun findTraitImplementation(descriptor: CallableMemberDescriptor): CallableMemberDescriptor? {
+    if (descriptor.getKind().isReal()) return null
+    if (CallResolverUtil.isOrOverridesSynthesized(descriptor)) return null
+
+    // TODO: this logic is quite common for bridge generation, find a way to abstract it to a single place
+    // TODO: don't use filterOutOverridden() here, it's an internal front-end utility (see its implementation)
+    val overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor)
+    val filteredOverriddenDeclarations = OverridingUtil.filterOutOverridden(LinkedHashSet(overriddenDeclarations))
+
+    var implementation: CallableMemberDescriptor? = null
+    for (overriddenDeclaration in filteredOverriddenDeclarations) {
+        if (DescriptorUtils.isTrait(overriddenDeclaration.getContainingDeclaration()) && overriddenDeclaration.getModality() != Modality.ABSTRACT) {
+            assert(implementation == null) { "Ambiguous overridden declaration: $descriptor" }
+            implementation = overriddenDeclaration
+        }
+    }
+    if (implementation == null) {
+        return null
+    }
+
+    // If this implementation is already generated into one of the superclasses, we need not generate it again, it'll be inherited
+    val containingClass = descriptor.getContainingDeclaration() as ClassDescriptor
+    val implClassType = implementation!!.getExpectedThisObject()!!.getType()
+    for (supertype in containingClass.getDefaultType().getConstructor().getSupertypes()) {
+        if (!DescriptorUtils.isTrait(supertype.getConstructor().getDeclarationDescriptor()!!) &&
+            TypeUtils.getAllSupertypes(supertype).contains(implClassType)) {
+            return null
+        }
+    }
+
+    return implementation
 }

compiler/backend/src/org/jetbrains/jet/codegen/ClassBodyCodegen.java

@@ -18,6 +18,7 @@ package org.jetbrains.jet.codegen;
 
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jet.codegen.bridges.BridgesPackage;
 import org.jetbrains.jet.codegen.context.ClassContext;
 import org.jetbrains.jet.codegen.state.GenerationState;
 import org.jetbrains.jet.lang.descriptors.*;
@@ -103,7 +104,7 @@ public abstract class ClassBodyCodegen extends MemberCodegen {
             for (DeclarationDescriptor memberDescriptor : descriptor.getDefaultType().getMemberScope().getAllDescriptors()) {
                 if (memberDescriptor instanceof FunctionDescriptor) {
                     FunctionDescriptor member = (FunctionDescriptor) memberDescriptor;
-                    if (member.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) {
+                    if (!member.getKind().isReal() && BridgesPackage.findTraitImplementation(member) == null) {
                         functionCodegen.generateBridges(member);
                     }
                 }

compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java

@@ -41,6 +41,7 @@ import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor;
 import org.jetbrains.jet.lang.psi.JetNamedFunction;
 import org.jetbrains.jet.lang.resolve.BindingContext;
 import org.jetbrains.jet.lang.resolve.DescriptorUtils;
+import org.jetbrains.jet.lang.resolve.calls.CallResolverUtil;
 import org.jetbrains.jet.lang.resolve.constants.ArrayValue;
 import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant;
 import org.jetbrains.jet.lang.resolve.constants.JavaClassValue;
@@ -425,6 +426,9 @@ public class FunctionCodegen extends ParentCodegenAwareImpl {
         if (owner.getContextKind() == OwnerKind.TRAIT_IMPL) return;
         if (isTrait(descriptor.getContainingDeclaration())) return;
 
+        // If the function doesn't have a physical declaration among super-functions, it's a SAM adapter or alike and doesn't need bridges
+        if (CallResolverUtil.isOrOverridesSynthesized(descriptor)) return;
+
         Set<Bridge<Method>> bridgesToGenerate = BridgesPackage.generateBridgesForFunctionDescriptor(
                 descriptor,
                 new Function1<FunctionDescriptor, Method>() {

compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java

@@ -17,15 +17,14 @@
 package org.jetbrains.jet.codegen;
 
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import com.intellij.openapi.progress.ProcessCanceledException;
-import com.intellij.openapi.util.Pair;
 import com.intellij.util.ArrayUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.codegen.binding.CalculatedClosure;
 import org.jetbrains.jet.codegen.binding.CodegenBinding;
 import org.jetbrains.jet.codegen.binding.MutableClosure;
+import org.jetbrains.jet.codegen.bridges.BridgesPackage;
 import org.jetbrains.jet.codegen.context.ClassContext;
 import org.jetbrains.jet.codegen.context.ConstructorContext;
 import org.jetbrains.jet.codegen.context.MethodContext;
@@ -1464,11 +1463,14 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
     private void generateTraitMethods() {
         if (JetPsiUtil.isTrait(myClass)) return;
 
-        for (Pair<CallableMemberDescriptor, CallableMemberDescriptor> pair : getTraitImplementations(descriptor)) {
-            CallableMemberDescriptor inheritedMember = pair.first;
-            CallableMemberDescriptor traitMember = pair.second;
+        for (DeclarationDescriptor declaration : descriptor.getDefaultType().getMemberScope().getAllDescriptors()) {
+            if (!(declaration instanceof CallableMemberDescriptor)) continue;
 
-            assert traitMember.getModality() != Modality.ABSTRACT : "Cannot delegate to abstract trait method: " + pair;
+            CallableMemberDescriptor inheritedMember = (CallableMemberDescriptor) declaration;
+            CallableMemberDescriptor traitMember = BridgesPackage.findTraitImplementation(inheritedMember);
+            if (traitMember == null) continue;
+
+            assert traitMember.getModality() != Modality.ABSTRACT : "Cannot delegate to abstract trait method: " + inheritedMember;
 
             // inheritedMember can be abstract here. In order for FunctionCodegen to generate the method body, we're creating a copy here
             // with traitMember's modality
@@ -1810,70 +1812,6 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
         }
     }
 
-
-    /**
-     * Return pairs of descriptors. First is member of this that should be implemented by delegating to trait,
-     * second is member of trait that contain implementation.
-     */
-    @NotNull
-    private static List<Pair<CallableMemberDescriptor, CallableMemberDescriptor>> getTraitImplementations(@NotNull ClassDescriptor classDescriptor) {
-        List<Pair<CallableMemberDescriptor, CallableMemberDescriptor>> result = Lists.newArrayList();
-
-        for (DeclarationDescriptor declaration : classDescriptor.getDefaultType().getMemberScope().getAllDescriptors()) {
-            if (!(declaration instanceof CallableMemberDescriptor)) continue;
-
-            CallableMemberDescriptor callableMemberDescriptor = (CallableMemberDescriptor) declaration;
-
-            CallableMemberDescriptor implementation = findTraitImplementation(callableMemberDescriptor);
-            if (implementation != null) {
-                result.add(Pair.create(callableMemberDescriptor, implementation));
-            }
-        }
-
-        return result;
-    }
-
-    /**
-     * Given a fake override descriptor, returns an overridden non-abstract descriptor whose container is a trait
-     */
-    @Nullable
-    private static CallableMemberDescriptor findTraitImplementation(@NotNull CallableMemberDescriptor descriptor) {
-        if (descriptor.getKind().isReal()) return null;
-
-        if (CallResolverUtil.isOrOverridesSynthesized(descriptor)) return null;
-
-        Collection<CallableMemberDescriptor> overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor);
-
-        Collection<CallableMemberDescriptor> filteredOverriddenDeclarations =
-                OverridingUtil.filterOutOverridden(Sets.newLinkedHashSet(overriddenDeclarations));
-
-        int count = 0;
-        CallableMemberDescriptor implementation = null;
-
-        for (CallableMemberDescriptor overriddenDeclaration : filteredOverriddenDeclarations) {
-            if (isTrait(overriddenDeclaration.getContainingDeclaration()) && overriddenDeclaration.getModality() != Modality.ABSTRACT) {
-                implementation = overriddenDeclaration;
-                count++;
-            }
-        }
-        if (implementation == null) {
-            return null;
-        }
-
-        assert count == 1 : "Ambiguous overridden declaration: " + descriptor;
-
-        ClassDescriptor containingClass = (ClassDescriptor) descriptor.getContainingDeclaration();
-        for (JetType supertype : containingClass.getDefaultType().getConstructor().getSupertypes()) {
-            //noinspection ConstantConditions
-            if (!isTrait(supertype.getConstructor().getDeclarationDescriptor()) &&
-                TypeUtils.getAllSupertypes(supertype).contains(implementation.getExpectedThisObject().getType())) {
-                return null;
-            }
-        }
-
-        return implementation;
-    }
-
     public void addClassObjectPropertyToCopy(PropertyDescriptor descriptor, Object defaultValue) {
         if (classObjectPropertiesToCopy == null) {
             classObjectPropertiesToCopy = new ArrayList<PropertyAndDefaultValue>();

compiler/testData/codegen/box/bridges/complexTraitImpl.kt

@@ -0,0 +1,28 @@
+import java.util.ArrayList
+import java.util.Arrays
+
+abstract class A {
+    abstract fun foo(): List<String>
+}
+
+trait B {
+    fun foo(): ArrayList<String> = ArrayList(Arrays.asList("B"))
+}
+
+open class C : A(), B
+
+trait D {
+    fun foo(): Collection<String>
+}
+
+class E : D, C()
+
+fun box(): String {
+    val e = E()
+    var r = e.foo()[0]
+    r += (e : D).foo().iterator().next()
+    r += (e : C).foo()[0]
+    r += (e : B).foo()[0]
+    r += (e : A).foo()[0]
+    return if (r == "BBBBB") "OK" else "Fail: $r"
+}

compiler/testData/codegen/box/bridges/fakeOverrideInTraitWithRequiredFromTraitImpl.kt

@@ -0,0 +1,17 @@
+abstract class A {
+    abstract fun foo(): Any
+}
+
+trait B {
+    fun foo(): String = "B"
+}
+
+trait C : A, B
+
+class D : A(), C
+
+fun box(): String {
+    val d = D()
+    val r = d.foo() + (d : C).foo() + (d : B).foo() + (d : A).foo()
+    return if (r == "BBBB") "OK" else "Fail: $r"
+}

compiler/testData/codegen/box/bridges/fakeOverrideOfPropertySetterInTraitImpl.kt

@@ -0,0 +1,23 @@
+var result = ""
+
+trait A {
+    var foo: String
+        get() = result
+        set(value) {
+            result += value
+        }
+}
+
+abstract class B {
+    abstract var foo: Any
+}
+
+class C : A, B()
+
+fun box(): String {
+    val c = C()
+    c.foo = "1"
+    (c : B).foo = "2"
+    (c : A).foo = "3"
+    return if (result == "123") "OK" else "Fail: $result"
+}

compiler/testData/codegen/box/bridges/fakeOverrideWithImplementationInTrait.kt

@@ -0,0 +1,18 @@
+trait A {
+    fun foo(): String = "A"
+}
+
+trait B {
+    fun foo(): Any
+}
+
+class C : A, B
+
+fun box(): String {
+    val c = C()
+    var result = ""
+    result += c.foo()
+    result += (c : B).foo()
+    result += (c : A).foo()
+    return if (result == "AAA") "OK" else "Fail: $result"
+}

compiler/testData/codegen/box/bridges/kt318.kt

@@ -0,0 +1,23 @@
+var result = ""
+
+trait Base
+open class Child : Base
+
+trait A<T : Base> {
+    fun <E : T> foo(a : E) {
+        result += "A"
+    }
+}
+
+class B : A<Child> {
+    override fun <E : Child> foo(a : E) {
+        result += "B"
+    }
+}
+
+fun box(): String {
+    val b = B()
+    b.foo(Child())
+    (b : A<Child>).foo(Child())
+    return if (result == "BB") "OK" else "Fail: $result"
+}

compiler/testData/codegen/box/bridges/noBridgeOnMutableCollectionInheritance.kt

@@ -0,0 +1,22 @@
+import java.util.ArrayList
+import java.util.Arrays
+
+trait A {
+    fun foo(): Collection<String>
+}
+
+trait B : A {
+    override fun foo(): MutableCollection<String>
+}
+
+class C : B {
+    override fun foo(): MutableList<String> = ArrayList(Arrays.asList("C"))
+}
+
+fun box(): String {
+    val c = C()
+    var r = c.foo().iterator().next()
+    r += (c : B).foo().iterator().next()
+    r += (c : A).foo().iterator().next()
+    return if (r == "CCC") "OK" else "Fail: $r"
+}

compiler/testData/codegen/box/bridges/traitImplInheritsTraitImpl.kt

@@ -0,0 +1,15 @@
+trait A {
+    fun foo(): Any = "A"
+}
+
+trait B : A {
+    override fun foo(): String = "B"
+}
+
+class C : B
+
+fun box(): String {
+    val c = C()
+    var r = c.foo() + (c : B).foo() + (c : A).foo()
+    return if (r == "BBB") "OK" else "Fail: $r"
+}

compiler/testData/codegen/box/bridges/traitWithRequiredCovariantOverride.kt

@@ -0,0 +1,15 @@
+open class A {
+    open fun foo(): Any = "A"
+}
+
+trait B : A {
+    override fun foo(): String = "B"
+}
+
+class C : A(), B
+
+fun box(): String {
+    val c = C()
+    val r = c.foo() + (c : B).foo() + (c : A).foo()
+    return if (r == "BBB") "OK" else "Fail: $r"
+}

compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java

@@ -290,6 +290,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest("compiler/testData/codegen/box/bridges/complexMultiInheritance.kt");
         }
         
+        @TestMetadata("complexTraitImpl.kt")
+        public void testComplexTraitImpl() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/complexTraitImpl.kt");
+        }
+        
         @TestMetadata("delegation.kt")
         public void testDelegation() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/delegation.kt");
@@ -315,11 +320,26 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest("compiler/testData/codegen/box/bridges/fakeGenericCovariantOverride.kt");
         }
         
+        @TestMetadata("fakeOverrideInTraitWithRequiredFromTraitImpl.kt")
+        public void testFakeOverrideInTraitWithRequiredFromTraitImpl() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/fakeOverrideInTraitWithRequiredFromTraitImpl.kt");
+        }
+        
+        @TestMetadata("fakeOverrideOfPropertySetterInTraitImpl.kt")
+        public void testFakeOverrideOfPropertySetterInTraitImpl() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/fakeOverrideOfPropertySetterInTraitImpl.kt");
+        }
+        
         @TestMetadata("fakeOverrideOfTraitImpl.kt")
         public void testFakeOverrideOfTraitImpl() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/fakeOverrideOfTraitImpl.kt");
         }
         
+        @TestMetadata("fakeOverrideWithImplementationInTrait.kt")
+        public void testFakeOverrideWithImplementationInTrait() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/fakeOverrideWithImplementationInTrait.kt");
+        }
+        
         @TestMetadata("fakeOverrideWithSeveralSuperDeclarations.kt")
         public void testFakeOverrideWithSeveralSuperDeclarations() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/fakeOverrideWithSeveralSuperDeclarations.kt");
@@ -360,6 +380,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest("compiler/testData/codegen/box/bridges/kt2920.kt");
         }
         
+        @TestMetadata("kt318.kt")
+        public void testKt318() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/kt318.kt");
+        }
+        
         @TestMetadata("longChainOneBridge.kt")
         public void testLongChainOneBridge() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/longChainOneBridge.kt");
@@ -375,6 +400,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest("compiler/testData/codegen/box/bridges/methodFromTrait.kt");
         }
         
+        @TestMetadata("noBridgeOnMutableCollectionInheritance.kt")
+        public void testNoBridgeOnMutableCollectionInheritance() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/noBridgeOnMutableCollectionInheritance.kt");
+        }
+        
         @TestMetadata("overrideAbstractProperty.kt")
         public void testOverrideAbstractProperty() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/overrideAbstractProperty.kt");
@@ -440,6 +470,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest("compiler/testData/codegen/box/bridges/simpleUpperBound.kt");
         }
         
+        @TestMetadata("traitImplInheritsTraitImpl.kt")
+        public void testTraitImplInheritsTraitImpl() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/traitImplInheritsTraitImpl.kt");
+        }
+        
+        @TestMetadata("traitWithRequiredCovariantOverride.kt")
+        public void testTraitWithRequiredCovariantOverride() throws Exception {
+            doTest("compiler/testData/codegen/box/bridges/traitWithRequiredCovariantOverride.kt");
+        }
+        
         @TestMetadata("twoParentsWithDifferentMethodsTwoBridges.kt")
         public void testTwoParentsWithDifferentMethodsTwoBridges() throws Exception {
             doTest("compiler/testData/codegen/box/bridges/twoParentsWithDifferentMethodsTwoBridges.kt");

core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java

@@ -318,7 +318,8 @@ public class OverridingUtil {
      *
      * @see CallableMemberDescriptor.Kind#isReal()
      */
-    public static Collection<CallableMemberDescriptor> getOverriddenDeclarations(CallableMemberDescriptor descriptor) {
+    @NotNull
+    public static Collection<CallableMemberDescriptor> getOverriddenDeclarations(@NotNull CallableMemberDescriptor descriptor) {
         Map<ClassDescriptor, CallableMemberDescriptor> result = Maps.newHashMap();
         getOverriddenDeclarations(descriptor, result);
         return result.values();