IR: add optimization pass for equals call from object instance

[CommanderTvis]

Related issue: https://youtrack.jetbrains.com/issue/KT-18435

[dnpetrov]

No, this is wrong, because it can change the behavior of code in question. As we have discussed it before, it has nothing to do with the sealedness of the super class.

Example 1, sealed super class, can't transform object equality to identity equality:

sealed class N(val value: Int) {
    override fun equals(other: Any?): Boolean {
        return other is N && value == other.value
    }
}

class I(value: Int) : N(value)

object Z : N(0)

sealed class N(val value: Int) {
    override fun equals(other: Any?): Boolean {
        return other is N && value == other.value
    }
}

class I(value: Int) : N(value)

object Z : N(0)

fun test(x: N) =
    when (x) {
        I(42) -> 42
        Z -> 0
        else -> -1
    }

Example 2, non-sealed class, can transform object equality to identity equality:

abstract class N(val value: Int)

class I(value: Int) : N(value)

object Z : N(0)

fun test(x: N) =
    when (x) {
        I(42) -> 42
        Z -> 0
        else -> -1
    }

If you really want to implement a transfromation like this, the following conditions must be met:

1. the class of the "left-hands side" of the object equality should have no custom equals method (otherwise it can do arbitrary things when compared to the given object);
2. the class of the "left-hands side" of the object equality should belong to the same module (so that if it gets a custom equals method, the comparison itself would be recompiled as well).

[udalov]

After implementing what @dnpetrov suggested, please add a test on incremental compilation to jps-plugin/testData/incremental/ which would check that after adding a custom equals to a class, its usage in another file in the same module is indeed recompiled. I'm a bit concerned that that might not happen automatically (which is what I was trying to explain in https://kotlinlang.slack.com/archives/C0BUHC9HD/p1624222629031400?thread_ts=1624041810.028500&cid=C0BUHC9HD).

[CommanderTvis]

Example 2, non-sealed class, can transform object equality to identity equality:

This is incorrect, because x may be something like:

class WrongN : N(42) { override fun equals(other: Any?) = false }

[CommanderTvis]

Checks recommended by @dnpetrov are done.

[CommanderTvis]

@udalov since the pass works only if the affected sealed class is in the same module with the call site, no special tests for incremental compilation are needed, aren't they?

[udalov]

No, I still have the concern discussed above: https://github.com/JetBrains/kotlin/pull/4496#issuecomment-878329065

Incremental compilation is a subsystem that works within one module, and it allows to avoid recompiling the whole module (= pass all source files of the module to the compiler) if not all source files were changed.

Suppose that you have a sealed class with an object subclass in one file, and an optimizable usage of == on that object in another file. You compile this code, and because of your optimization, there's no equals call in the bytecode, as it's been optimized to identity check.

Then, you add a custom equals to the object in the first file and compile the module again. In the first pass, only the first file will be passed to the compiler. Now, whether or not there will be a second pass depends on whether the incremental compilation can figure out that there's a dependency from the second file to the first. If it doesn't figure out that, the second file will not be recompiled, and the behavior at runtime will be incorrect.

It might be the case that it works as expected because of some aspect of incremental compilation which I don't know -- e.g. if the class is changed in any way, recompile all its usages in all files -- but I'm asking to add a test just to be sure.

[CommanderTvis]

Incremental compilation is a subsystem that works within one module, and it allows to avoid recompiling the whole module (= pass all source files of the module to the compiler) if not all source files were changed.

Now I understand.

[CommanderTvis]

No, I still have the concern discussed above: #4496 (comment)

Incremental compilation is a subsystem that works within one module, and it allows to avoid recompiling the whole module (= pass all source files of the module to the compiler) if not all source files were changed.

Suppose that you have a sealed class with an object subclass in one file, and an optimizable usage of == on that object in another file. You compile this code, and because of your optimization, there's no equals call in the bytecode, as it's been optimized to identity check.

Then, you add a custom equals to the object in the first file and compile the module again. In the first pass, only the first file will be passed to the compiler. Now, whether or not there will be a second pass depends on whether the incremental compilation can figure out that there's a dependency from the second file to the first. If it doesn't figure out that, the second file will not be recompiled, and the behavior at runtime will be incorrect.

It might be the case that it works as expected because of some aspect of incremental compilation which I don't know -- e.g. if the class is changed in any way, recompile all its usages in all files -- but I'm asking to add a test just to be sure.

I've added a test that ensures that use-site of optimizable == gets recompiled if a sealed subclass was changed.