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

Kotlin: Add 1.8 support #11637

Merged
merged 9 commits into from
Jan 12, 2023
Merged

Kotlin: Add 1.8 support #11637

merged 9 commits into from
Jan 12, 2023

Conversation

igfoo
Copy link
Contributor

@igfoo igfoo commented Dec 9, 2022

No description provided.

@rotilho
Copy link

rotilho commented Dec 28, 2022

Kotlin 1.8 was just released :)

@henrik242
Copy link

Any progress on this? :)

igfoo added 9 commits January 10, 2023 14:41
The build no longer works for Kotlin < 1.8: We get

    error: class 'org.jetbrains.kotlin.ir.IrElement' was compiled
           with an incompatible version of Kotlin. The binary version
           of its metadata is 1.8.0, expected version is 1.6.0.
We now get better locations, with Kotlin 1.8.0.
We get better locations with Kotlin 1.8.0.
@igfoo igfoo marked this pull request as ready for review January 10, 2023 20:44
@igfoo igfoo requested review from a team as code owners January 10, 2023 20:44
@marcphilipp marcphilipp mentioned this pull request Jan 11, 2023
7 tasks
# 21| 1: [Method] a
#-----| 1: (Annotations)
# 21| 1: [Annotation] JvmName
# 0| 1: [StringLiteral] "a"
# 21| 1: [StringLiteral] "a"
Copy link
Contributor

Choose a reason for hiding this comment

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

That is the reason for these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kotlin is now giving the components of annotations locations, so we are getting a line number (21) for this.

Comment on lines -211 to +218
# 0| -1: [TypeAccess] Y
# 0| 5: [ArrayInit] {...}
# 0| 1: [VarAccess] Y.C
# 0| -1: [TypeAccess] Y
# 0| 2: [VarAccess] Y.A
# 0| -1: [TypeAccess] Y
# 0| 6: [Annotation] Annot0k
# 0| 4: [Annotation] Annot0k
# 0| 1: [IntegerLiteral] 1
# 39| 5: [VarAccess] Y.B
# 39| -1: [TypeAccess] Y
# 39| 6: [ArrayInit] {...}
# 39| 1: [VarAccess] Y.C
# 39| -1: [TypeAccess] Y
# 39| 2: [VarAccess] Y.A
# 39| -1: [TypeAccess] Y
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the order changed here. Why? Is it expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just because PrintAst uses locations to order by

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Yes, you're right. AnnotationsNode is ordering child elements by file, line, column, element.toString().

@igfoo igfoo merged commit 9ebe59d into github:main Jan 12, 2023
@igfoo igfoo deleted the igfoo/kotlin-1.8 branch January 12, 2023 12:15
@marcphilipp
Copy link

@tamasvajk @igfoo When can this be used in GitHub Actions?

@igfoo
Copy link
Contributor Author

igfoo commented Jan 19, 2023

We expect it will be in the default actions image by 15th February.

@jean-andre-gauthier
Copy link

@tamasvajk @igfoo I was wondering if there was any reason that prevents CodeQL from supporting newer/upcoming Kotlin patch versions? We've recently upgraded to Kotlin 1.8.10 in one of our builds, but unfortunately it looks like CodeQL supports only the versions below 1.8.10

cc @blindpirate

@igfoo
Copy link
Contributor Author

igfoo commented Feb 6, 2023

Unfortunately, versions that change the 10s digit of the patch version tend to have minor changes to the compiler's implementation, which require corresponding changes to our plugin.

trask added a commit to open-telemetry/opentelemetry-java-instrumentation that referenced this pull request Feb 10, 2023
Codeql doesn't support 1.8.10 yet:
github/codeql#11637 (comment)

but it does support 1.8.0 now
FirelightFlagboy added a commit to Scille/parsec-cloud that referenced this pull request Mar 13, 2023
We need to revert to `kotlin-1.8.0` because `CodeQL` don't currently support a version `>= 1.8.10`

```txt
Kotlin version 1.8.10 is too recent. CodeQL currently supports versions below 1.8.10
```

For more information see: <github/codeql#11637 (comment)>

closes #4216
FirelightFlagboy added a commit to Scille/parsec-cloud that referenced this pull request Mar 13, 2023
We need to revert to `kotlin-1.8.0` because `CodeQL` don't currently support a version `>= 1.8.10`

```txt
Kotlin version 1.8.10 is too recent. CodeQL currently supports versions below 1.8.10
```

For more information see: <github/codeql#11637 (comment)>

closes #4217
FirelightFlagboy added a commit to Scille/parsec-cloud that referenced this pull request Mar 13, 2023
We need to revert to `kotlin-1.8.0` because `CodeQL` don't currently support a version `>= 1.8.10`

```txt
Kotlin version 1.8.10 is too recent. CodeQL currently supports versions below 1.8.10
```

For more information see: <github/codeql#11637 (comment)>

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

Successfully merging this pull request may close these issues.

6 participants