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

Fix 2.13.15-only false positives with -Wunused:patvars #10870

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Sep 26, 2024

Fixes false positives from -Wunused:patvars in the presence of filters (if) — this was scala/bug#13041.

Note that -Wunused:patvars is part of -Wunused but is not enabled by -Xlint. With this fix, interested users may wish to make a fresh attempt to add -Wunused:patvars to their builds.

The now-correct position information may also aid tooling, as seen in scala/bug#13037 .

Followup to #10812, which (in 2.13.15) expanded unused checking into this area.

Fixes scala/bug#13041
Fixes scala/bug#13037

Implementation notes

Summary: Track patvars by position (and correct span of guard closure)

The scheme of linking duplicated patvar trees via attachments is brittle because links are stale after duplication; in particular, they point to old trees which may not be typechecked, so symbols are not recoverable.

Instead, just identify associated trees by position: duplicates have a focused position and the "original" has its opaque range.

Fix range of guard closure in TreeGen.mkFor to include parameter. (In the previous scheme, the pattern and its duplicate were swapped to build the "links", but now have been swapped back again, so that bug is nullified.)

Some mild refactoring of position validation for readability.

@scala-jenkins scala-jenkins added this to the 2.13.16 milestone Sep 26, 2024
@som-snytt som-snytt marked this pull request as ready for review September 26, 2024 16:46
@som-snytt som-snytt changed the title Correct span of guard closure Track patvars by position (and correct span of guard closure) Sep 29, 2024
@som-snytt som-snytt marked this pull request as draft September 29, 2024 03:05
@som-snytt som-snytt marked this pull request as ready for review September 29, 2024 20:40
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Nice, simpler and correcter. Thank you!

@lrytz lrytz merged commit ee9a74f into scala:2.13.x Oct 9, 2024
4 checks passed
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Oct 9, 2024
@SethTisue
Copy link
Member

@som-snytt I added a release-note, hopefully an accurate one

@SethTisue SethTisue changed the title Track patvars by position (and correct span of guard closure) Fix 2.13.15-only false positives with -Wunused:patvars Oct 9, 2024
@som-snytt som-snytt deleted the followup/tree-pos-linted branch October 9, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
4 participants