Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIR] Resolve annotations inside contracts #5311

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isuckatcs
Copy link
Contributor

^KT-68893 Fixed

Inside a contract annotations are not resolved which results in an internal error.

I might be missing something, but seemingly both contract and bar are functions in the language, so I'm not sure why we need a different rule for annotations within contracts.

fun bar(block: () -> Unit) {
    block();
}

fun foo() {
    contract {
        @foo // fun contact() -> @foo
    }
    
    bar {
        @foo // fun bar() -> @foo
    }
}

I haven't seen any new tests failing by removing the overrides. It might be possible though that now the CFG contrains edges, it shouldn't. In that case I guess, we can just resolve the type of the annotation in the overridden methods and return.

@demiurg906
Copy link
Contributor

Hi @isuckatcs. Thank you for the contribution, but your change is incorrect

FirContractResolveTransformer is used to transform all elements in file, and it is intended to transform only contract declarations on functions, without touching any other elements in the tree. So if you remove those overloads for transformAnnotation, annotations outside contracts will start to be transformed on CONTRACTS phase instead of BODY_RESOLVE/IMPLICIT_BODY_RESOLVE

So the proper solution would be to differentiate annotations inside contract blocks and outside of them

Also it's incorrect to just resolve the annotation regularly as on body resolve phase, as on contracts phase there is no guarantee that all callables have return types resolved, so code like this most likely will fail (probably in a non-trivial way)

annotation class Ann(val x: String)

fun foo(b: Boolean) {
    contract {
        @Ann(s) returns() implies b
    }
}

const val s = "hello" // no explicit return type

Potentially it can be solved by special resolution mechanism for arguments of such annotations, which will just replace all of them with error nodes with proper diagnostics. If you want, you can try to do it

@demiurg906
Copy link
Contributor

There is one moment about contributions to the compiler: by default we consider only tickets with up-for-grabs tag. If you feel that there is some issue which you really can and want to fix which don't contain this tag, please discuss it in the issue itself before starting a pull request (there are might be some non-trivial constraints which are not obvious at first glance if you don't have enough context). Otherwise your PR may be rejected

Copy link
Contributor

@demiurg906 demiurg906 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*I've described everything above, but github doesn't allow me to press "Submit review" without an additional comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants