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

Improve message for discarded pure non-Unit values #18723

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Oct 19, 2023

Fixes #18408
Fixes #18722

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

does this also work for #18408?

@nicolasstucki
Copy link
Contributor Author

does this also work for #18408?

Yes, it also fixes that issue. I will add it to the tests.

@nicolasstucki nicolasstucki force-pushed the fix-18722 branch 3 times, most recently from a057274 to d82a328 Compare October 20, 2023 07:51
@bishabosha
Copy link
Member

does this still need to be a draft?

@nicolasstucki nicolasstucki marked this pull request as ready for review October 20, 2023 08:43
@nicolasstucki
Copy link
Contributor Author

does this still need to be a draft?

I added a second commit with some extra cleanups. Now it is ready for review.

@@ -1,7 +1,7 @@
-- [E129] Potential Issue Warning: tests/neg-macros/annot-suspend-cycle/Macro.scala:7:4 --------------------------------
7 | new Foo
| ^^^^^^^
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
| A pure expression does nothing in statement position
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one might not be correct. I will double check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is correct. Class Foo has a pure constructor.

The `you may be omitting necessary parentheses` hint in the message was
added when we used to get method references that became lambdas in statement
positions. Now these are an error and never reach this warning.
@nicolasstucki nicolasstucki merged commit 5af78d8 into scala:main Oct 20, 2023
15 checks passed
@nicolasstucki nicolasstucki deleted the fix-18722 branch October 20, 2023 14:17
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 2023
WojciechMazur added a commit that referenced this pull request Jun 23, 2024
…20731)

Backports #18723 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants