Use resolvedStatus in FirOverrideChecker

[ZacSweers]

Currently, FirOverrideChecker uses member.isOverride, which internally only checks against rawStatus. This results in this checker missing any statuses transformed by a FirStatusTransformerExtension, for example one that sets isOverride to true for a member that matches the signature of a member inherited from a contributed supertype in a FirSupertypeGenerationExtension.

See https://github.com/ZacSweers/metro/pull/584

[cypressious]

Could you please add a test, e.g., in plugins/plugin-sandbox.

[ZacSweers]

Done! I wasn't sure about where to add tests or if that was the right place, let me know if there's another place I should add to instead

[cypressious]

Looks good, I'm running the CI.

[cypressious]

Running your added test with org.jetbrains.kotlin.analysis.low.level.api.fir.diagnostic.compiler.based.LLSandboxBackBoxTestGenerated fails with a StackOverlflowError. Would you like to try and fix it yourself?

[ZacSweers]

I can try. Do you know what's different about that test suite's behavior?

[cypressious]

It runs in Analysis API mode. Declarations might be lazily resolved whereas in CLI mode they're resolved eagerly.

[ZacSweers]

@cypressious I'm not sure how to get those tests running locally so I might need to turn this over to you. When I try to run from the IDE I get this

Lib with annotations does not exist. Please run :plugins:plugin-sandbox:plugin-annotations:distAnnotations or specify firPluginAnnotations.path system property
java.lang.IllegalStateException: Lib with annotations does not exist. Please run :plugins:plugin-sandbox:plugin-annotations:distAnnotations or specify firPluginAnnotations.path system property
	at org.jetbrains.kotlin.plugin.sandbox.PluginAnnotationsProviderKt.findLib(PluginAnnotationsProvider.kt:66)
	at org.jetbrains.kotlin.plugin.sandbox.PluginAnnotationsProviderKt.findJvmLib(PluginAnnotationsProvider.kt:55)
	at org.jetbrains.kotlin.plugin.sandbox.PluginAnnotationsProviderKt.access$findJvmLib(PluginAnnotationsProvider.kt:1)
	at org.jetbrains.kotlin.plugin.sandbox.PluginAnnotationsProvider.configureCompilerConfiguration(PluginAnnotationsProvider.kt:26)

[demiurg906]

@ZacSweers have you run ./gradlew :plugins:plugin-sandbox:plugin-annotations:distAnnotations, as the failure suggests?

[cypressious]

Could you please also

1. Create a KT ticket for this problem

2. Reorganize exsiting commits into the following structure

[Test] Reproduce KT-XXXXX

[FIR] ... your commit message ...

^KT-XXXXX Fixed

3. Send fixes to existing commits as fixups (`git commit --fixup <target commit>`)

https://youtrack.jetbrains.com/issue/KT-78351/FirOverrideChecker-ignores-status-modified-by-plugin

[ZacSweers]

Send fixes to existing commits as fixups

As in you want them all to be squashed to one commit?

[ZacSweers]

@ZacSweers have you run ./gradlew :plugins:plugin-sandbox:plugin-annotations:distAnnotations, as the failure suggests?

ah I missed that this was supposed to be a gradle command to run first. Up and running now. It looks like the overflow is coming from the simple supertype generator I added when it does a simple predicate match. I'm not sure off-hand why this would trip up this mode, would one of y'all know?

[demiurg906]

As in you want them all to be squashed to one commit?

Yeah. It makes the review much easier, as it allows distinguishing between the original changes and ones regarding MR comments. And then such fixups are automatically squashed with targets by our merge robot.

Here is an example of one of my personal branches on review: rr/demiurg906/dynamic-operators

I'm not sure off-hand why this would trip up this mode, would one of y'all know?

Hard to say without a particular test and some debugging actually. My wild guess is that supertype generation extension could be run for typealiases for some reason, which leads to problems during expansion, but I'm not sure. Could you please investigate this further and describe the actual loop which leads to SOF? I'm afraid that it could be an indicator of some problem inside the compiler/AA.

[cypressious]

I've fixed the failing test and pushed the changes to rr/krakhman/plugin_override_checker.

[ZacSweers]

Thanks! I haven't had a chance to revisit yet as I was busy with a conference all last week. I can pull your fix into my branch and update this today

[cypressious]

There's no need, I'll merge the branch directly. However, there are still some other tests failing which I'll investigate today.

[ZacSweers]

Much appreciated 🙇

[cypressious]

The fix is being merged.