https://kotlinlang.org logo
i

Iaroslav Postovalov

06/18/2021, 6:43 PM
@mglukhikh Hello! I'm working on KT-18435, and I've started implementing it as an IR lowering phase for == calls. My problem is that
IrClass
doesn't have
sealedSubclasses
. I see only two options to solve this problem: 1. Publish
IrBuiltIns.toIrSymbol
function to use
ClassDescriptor.getSealedSubclasses
(and it seems to be a bad way). 2. Add such property and support it in
fir2ir
and
psi2ir
. I need some opinions from the compiler team to get if I can add this property or this problem can be solved in another way.
cc @Ilmir Usmanov [JB]
d

dmitriy.novozhilov

06/18/2021, 8:34 PM
cc @udalov
Option 1 is definitely bad, because we want to get rid of descriptors from IR at all. Option 2 looks ok from frontend perspective, but I think we need to hear opinion from someone from backend teams
i

Ilmir Usmanov [JB]

06/19/2021, 12:50 AM
cc @dmitry.petrov for psi2ir expertise.
i

Iaroslav Postovalov

06/20/2021, 11:56 AM
😭
u

udalov

06/20/2021, 8:27 PM
Can you clarify how exactly you are going to fix this issue, i.e. how you’d use
sealedSubclasses
here?
Adding
IrClass.sealedSubclasses
should be OK but it might be an overkill for KT-18435 which is basically a minor performance optimization.
i

Iaroslav Postovalov

06/20/2021, 8:32 PM
Let us have a sealed class or interface. If in
==
the left operand is an instance of that class and the right one is an
object
instance, moreover no subtypes of our sealed class
override
equals
, then we can replace
==
with
===
.
So, I need
sealedSubclasses
to check whether any of subtypes has custom
equals
.
@udalov
u

udalov

06/20/2021, 8:57 PM
moreover no subtypes of our sealed class override 
equals
Note that this won’t work out of the box in terms of partial/incremental compilation. You can only safely assume anything like that about declarations in the source files that you’re compiling. If the sealed class is in another module, or even in the same module but compiled on a previous round of incremental compilation, you can’t rely that there’s no custom
equals
in its hierarchy. Because it might be added or removed independently from the use site code, which will change runtime behavior. It’s not directly related to your question, I’m just trying to process if this optimization would be worth it at all. So far it looks like it needs to be implemented very carefully: even if you have
IrClass.sealedSubclasses
, you need to check that all of them are declared in a source file, and you need a separate change in incremental compilation machinery to make sure that once a user adds
equals
to a hierarchy, all call sites that use
==
are invalidated. And the latter seems rather complicated for a minor performance optimization.
@dmitry.petrov
has been pinged above, I’d also wait for his take on this issue. :)
i

Iaroslav Postovalov

06/21/2021, 5:28 AM
Well, there aren't so many up-for-grabs issues, so I think this one is OK
d

dmitry.petrov

06/21/2021, 5:31 AM
From the first glance, optimization you are trying to implement actually looks dangerous in the scope of separate compilation. A class, sealed or not, might get a user-defined
equals
method later. Replacing equality with reference equality thus would change the behavior here. The fact that the given class is sealed doesn't seem to be relevant.
So, strictly speaking, it has nothing to do with a class on the left side of equality being sealed, or
when
being sealed. Class should belong to the same module and should have no user-defined
equals
(should have
equals
inherited from
Any
).
i

Iaroslav Postovalov

06/21/2021, 12:04 PM
I know, my current code which optimizes
Object == something
already checks it.
The "sealedness" of class is important because if left operand isn't sealed, it is undefined whether it has user-defined
equals
cc @snrostov in the context of incremental compilation
d

dmitry.petrov

06/21/2021, 1:05 PM
The "sealedness" of class is important because if left operand isn't sealed, it is undefined whether it has user-defined 
equals
No. Both sealed and non-sealed classes can have user-defined
equals
. It's important whether we can definitely say that the class of the left operand doesn't have a user-defined
equals
. It also should belong to the same module (so that it would be recompiled together with the given code). Otherwise we can't replace object equality with reference equality. It's a frequent misconception that sealed classes are true algebraic data types. Unfortunately, they are not (even though it's a good idea to implement an algebraic data type using a sealed class), because of possibility of user-defined
equals
. Such user-defined
equals
can do whatever it wants, because it can.
i

Iaroslav Postovalov

06/21/2021, 1:08 PM
In case of sealed classes we can just check all the subclasses of it, can't we?
2 Views