Skip to content

De-anonymize patvar in given pattern #23121

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

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

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented May 8, 2025

Fixes #23119

A given pattern in a for comprehension results in a fresh val in the body of the mapping function, but is not correlated with patvars.

This commit gives the given its usual given name (in makeIdPat) so that the unused check can check it.

A subsequent pattern var is named when typechecked, which resulted in the warning.

The fix adds the PatternVar attachment in makeIdPat and also fixes the tree pos, so that it corresponds to the position of the pat var in the elaboration of the for (that is, the position of the given T tree).

The unused check can now detect the patvar using the attachment and use the tree pos (instead of the bind name pos).

Adds some mild refactoring to name subexpressions and reduce line lengths.

@som-snytt som-snytt force-pushed the issue/23119-for-given branch from def4dd7 to 4d0f402 Compare May 9, 2025 06:19
@som-snytt som-snytt changed the title Detect anonymized patvar in given pattern De-anonymize patvar in given pattern May 9, 2025
@som-snytt
Copy link
Contributor Author

The analysis relies on ignoring derived names and also correlating pattern vars by position.

The existing artifact received a "fresh" name and a zero-extent synthetic position.

@som-snytt
Copy link
Contributor Author

The gods would often appear in disguise -- probably one may say more precisely that they were present but did not appear -- and in fact, the Easter story tells us that they didn't recognize Jesus, though maybe that was because they were not expecting him.

Did you know that the incognito gods dwell on Mount Anonympus?

unnamed

In English, the given name is a Vorname, Taufname, which, one discovers as a parent, is also invented.

@som-snytt som-snytt marked this pull request as ready for review May 9, 2025 06:55
@som-snytt som-snytt force-pushed the issue/23119-for-given branch from 4d0f402 to 1df2c1d Compare May 9, 2025 07:03
@som-snytt som-snytt force-pushed the issue/23119-for-given branch from 1df2c1d to c9a9dbc Compare May 9, 2025 07:08
@som-snytt
Copy link
Contributor Author

I would like to settle on conventions for "refining" patterns.

I changed names in the match:

      def makeIdPat(pat: Tree): (Tree, Ident) = pat match
        case pat @ Bind(nme.WILDCARD, body) =>
          val name =
            body match
            case Typed(Ident(nme.WILDCARD), tpt) if pat.mods.is(Given) => inventGivenName(tpt)
            case _ => UniqueName.fresh()
          cpy.Bind(pat)(name, body)
            .withMods(pat.mods)
          ->
          Ident(name)

where pat has the same referent and body refers to the nested tree and is the canonical name (if not descriptive here).

Previously, case bind @ Bind(x, pat) => was confusing. I would like a lint to tell me that it is confusing. All the symbols are "used", and it's normally ok to rebind pat to a subpattern, but maybe not if the original pat is also aliased.

@Gedochao Gedochao requested a review from tgodzik May 9, 2025 08:25
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.

false positive warning
2 participants