Postpone static field's companion resolution #251
Conversation
|
@martinbonnin PTAL |
| /* | ||
| * In certain cases (like with enum entries), the fact a field is both static and final does not | ||
| * imply it belongs to a companion object. | ||
| * We don't update field's `companionClass` until we're 100% sure the field was | ||
| * actually moved from the companion. | ||
| */ | ||
| companionClass = companionClassCandidate |
There was a problem hiding this comment.
What about not updating companionClass at all here? Looks like CompanionFieldBinarySignature is only to be used for the actual field referencing the companion?
The bug is me copy/pasting the code from the other branch without realising there would be a side effect at the end of the function :/
There was a problem hiding this comment.
In general, it seems reasonable to include companionClass so its visibility can be taken into account. But this part seems to be totally broken.
I'll remove the companionClass update and open a separate issue on proper const val handling.
The bug is me copy/pasting the code from the other branch without realising there would be a side effect at the end of the function :/
No worries, it's all about the way JVM ABI is built and filtered (to be more precise, how complicated it became at some point) in conjunction with the test coverage not being full.
|
LGTM thanks a lot for fixing this 🙏 |
#245 fixed the way
const vars from companion objects are handled. Unfortunately, the solution interferes with how any other static final fields are processed.This PR postpones companion class resolution until it's known that a field corresponds to a property declared in a companion object.
Fixes #250