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

KE2: Extract null literal #18127

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

KE2: Extract null literal #18127

wants to merge 1 commit into from

Conversation

tamasvajk
Copy link
Contributor

No description provided.

@tamasvajk tamasvajk requested a review from a team as a code owner November 27, 2024 09:49
val type = useType(t)
// Match Java by using a special <nulltype> for nulls, rather than Kotlin's view of this which is
// kotlin.Nothing?, the type that can only contain null.
val nullTypeName = "<nulltype>"
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't entirely consistent with Java, e.g.

public class Foo {
    void foo() {
        java.lang.Void v = true ? null : null;
    }
}

gives

| Foo.java:3:28:3:45 | ...?...:... | file://:0:0:0:0 | <nulltype> |
| Foo.java:3:35:3:38 | null | file://:0:0:0:0 | <nulltype> |
| Foo.java:3:42:3:45 | null | file://:0:0:0:0 | <nulltype> |

(I'm not sure we have a Java test for that; might be worth adding one)

I think what we want is to check for Nothing? in useType, and return nullType for the Java type there.

Copy link
Contributor

Choose a reason for hiding this comment

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

To check: I was recently requested to patch KE1 per #17890 to get the type of null consistent with Java

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, if Kotlin sees a function defined in Java as public java.lang.Void foo(), then does that look like a Nothing? return type in Kotlin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@igfoo, are you saying that KE2 is not consistent with KE1, or KE2/KE1 not consistent with the java extractor?

Copy link
Contributor

Choose a reason for hiding this comment

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

KE2/KE1 not consistent with the java extractor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's independent of KE2, then I think we should handle that inconsistency in a separate PR.

@tamasvajk
Copy link
Contributor Author

I rebased this PR to fix a merge conflict.

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

Successfully merging this pull request may close these issues.

3 participants