New footprint calculation scheme #19639
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
odersky
force-pushed
the
fix-matchtype-footprint
branch
from
21fb6cb to
84ace9e
Compare
The old one clearly did not work. It either never worked or was disabled by the changes to matchtype reduction. I now changed it to a more straightforward scheme that computes the footprint directly instead of relying on TypeComparer to produce the right trace.
# Conflicts: # compiler/src/dotty/tools/dotc/core/Types.scala
odersky
force-pushed
the
fix-matchtype-footprint
branch
from
84ace9e to
d6ba9b2
Compare
|
Looking at the code, I think the old footprint calculation probably did work, but was bypassed by the new matchtype implementation since it is now formulated in terms of baseType instead of subtyping constraints. |
sjrd
approved these changes
Feb 14, 2024
EugeneFlesselle
added a commit
to EugeneFlesselle/dotty
that referenced
this pull request
Mar 25, 2024
This is another case which used to result in an unreported `ErrorType` It is now detected only when enabling `-Yforce-sbt-phases -Xverify-signatures -Ycheck:all` The error comes from: ```scala type Names[X <: AnyNamedTuple] <: Tuple = X match case NamedTuple[n, _] => n ``` The `NamedTuple` pattern is legal as long as the type alias is opaque. Following elimOpaque however, it is beta-reduced (as normal applied type aliases in match type patterns) to a pattern which no longer contains an instance of the type capture `n`, making it an illegal pattern. This used to return an `ErrorType`, which happened not be reported as in the other cases. Also note this error could also be missed without the New footprint calculation scheme (scala#19639) Given the changes to the footprint calculation and failed reduction reporting, this is now the same issue as scala#19434.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since match type reduction is expensive, it is cached. If a type is reduced (or not reduced) in
tryNormalizewe remember that decision and return the previous result - unless something in the context changed since the last attempt which could lead to a different outcome. Relevant here are:We keep track of these things in a so-called footprint calculation.
The old calculation clearly did not work. It either never worked or was broken by the changes to matchtype reduction.
I now changed it to a more straightforward scheme that computes the footprint directly instead of relying on TypeComparer to produce the right trace.