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

Backport "Fix signature computation and caching involving type variables " to LTS #19107

Merged
merged 7 commits into from
Dec 8, 2023

Conversation

Kordyjan
Copy link
Contributor

Backports #18092 to the LTS branch.

PR submitted by the release tooling.
[skip ci]

Its meaning will be expanded in the next commit.

[Cherry-picked 84bc1bd]
During typechecking, we sometimes need to compute the signature of a type
involving uninstantiated type variables or wildcards (in particular, because
signature matching is doen as part of subtype checking for methods), this is
handled with `tpnme.Uninstantiated` which is handled specially in `Signature`.

Before this commit, `sigName` only checked for wildcards and type variables
at the top-level of the type, even though nested types can still have
an impact on type erasure, in particular when they appear as part of:
- an intersection
- a union
- an underlying type of a derived value class
- the element type of an array type
- an element type of a tuple (... *: X *: ...)

We keep track of all these situations by returning `null` in
`TypeErasure#apply` when encountering a wildcard or uninstantiated type
variable, which we then propage upwards to the `sigName` call. This propagation
only happens for `sigName` calls (where `inSigName` is true), otherwise we
throw an assertion since erasure shouldn't normally be computed for
underdefined types.

[Cherry-picked c21133f]
Signatures should not be cached if they may change. The existing
`isUnderDefined` check is not enough to guarantee this because the signature
might have been computed based on a type variable instantiated in a TyperState
which was subsequently retracted.

To guard against this, we now rely on `Type#isProvisional`. This is a stricter
check than needed since the provisional part of the type does not always have
an impact on its signature, but in practice this is fine: when compiling Dotty,
signature cache misses increased by less than 2%.

[Cherry-picked 3729960]
I'm afraid we'll have to get rid of it if we want our caches to be correct, but
I don't have the time to look into it right now.

[Cherry-picked 7de3663]
As requested in the review, this is slightly shorter but means we have to be
very careful to not accidentally forget to propagate a WildcardType to the
top-level when computing signatures.

[Cherry-picked 4499bc0]
Base automatically changed from lts-18719 to release-3.3.2 December 8, 2023 15:19
@Kordyjan
Copy link
Contributor Author

Kordyjan commented Dec 8, 2023

No regressions detected in the community build up to lts-16941.

Reference

@Kordyjan Kordyjan merged commit 76e39ce into release-3.3.2 Dec 8, 2023
19 checks passed
@Kordyjan Kordyjan deleted the lts-18092 branch December 8, 2023 15:19
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.

2 participants