Iaroslav Postovalov
06/18/2021, 6:43 PMIrClass
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.dmitriy.novozhilov
06/18/2021, 8:34 PMIlmir Usmanov [JB]
06/19/2021, 12:50 AMIaroslav Postovalov
06/20/2021, 11:56 AMudalov
sealedSubclasses
here?IrClass.sealedSubclasses
should be OK but it might be an overkill for KT-18435 which is basically a minor performance optimization.Iaroslav Postovalov
06/20/2021, 8:32 PM==
the left operand is an instance of that class and the right one is an object
instance, moreover no subtypes of our sealed classequals
, then we can replace ==
with ===
.sealedSubclasses
to check whether any of subtypes has custom equals
.udalov
moreover no subtypes of our sealed class overrideNote 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 customequals
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. :)Iaroslav Postovalov
06/21/2021, 5:28 AMdmitry.petrov
06/21/2021, 5:31 AMequals
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.when
being sealed.
Class should belong to the same module and should have no user-defined equals
(should have equals
inherited from Any
).Iaroslav Postovalov
06/21/2021, 12:04 PMObject == something
already checks it.equals
dmitry.petrov
06/21/2021, 1:05 PMThe "sealedness" of class is important because if left operand isn't sealed, it is undefined whether it has user-definedNo. Both sealed and non-sealed classes can have user-definedequals
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.Iaroslav Postovalov
06/21/2021, 1:08 PM