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

Correct occurences in for comprehensions containing withFilter #3109

Merged
merged 1 commit into from Apr 25, 2023

Conversation

susliko
Copy link
Contributor

@susliko susliko commented Apr 23, 2023

@@ -1487,8 +1487,7 @@ com/javacp/Test#staticField. => private[javacp] static field staticField: Int
com/javacp/Test#staticMethod(). => private[javacp] static method staticMethod(): Unit
javacp => com/javacp/
Unit => scala/Unit#
com/javacp/Test#strictfpMethod(). => @strictfp private[javacp] method strictfpMethod(): Unit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why change this? It seems to be failing now. You can use the save-expect command to update the expected file.

Copy link
Contributor Author

@susliko susliko Apr 24, 2023

Choose a reason for hiding this comment

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

That's the result of save-expect (even if invoked on main branch) with jvm 17+
Compilation also gives a warning:

[warn] /.../scalameta/semanticdb/integration/src/main/java/com/javacp/Test.java:69:1:  [strictfp] as of release 17, all floating-point expression
s are evaluated strictly and 'strictfp' is not required
[warn]     strictfp void strictfpMethod() {}
[warn]                   ^1 warning

I could switch to an older jvm version to leave this unchanged, should I?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the issue is that we run the tests with 8/11 then 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep
Another option is to remove this strictfpMethod from tests 😃

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can for sure change that, I think this was added as a test for annotations if I am not mistaken

Copy link
Contributor Author

@susliko susliko Apr 24, 2023

Choose a reason for hiding this comment

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

Not sure about annotations, it's defined here:

strictfp void strictfpMethod() {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

ach, that might be something different then, sorry I didn't look exactly. The best case here is to run sbt with JDK 11, if it's a problem for you I can do it later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a problem at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@susliko susliko force-pushed the fix-for-comp branch 2 times, most recently from b16e8b1 to 274cd32 Compare April 25, 2023 09:06
Copy link
Collaborator

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM!

@tgodzik tgodzik merged commit c88fadb into scalameta:main Apr 25, 2023
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go to definition breaks in for comprehensions containing .unapply calls
2 participants