Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only cache base types when gadt state is empty #19562

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

EugeneFlesselle
Copy link
Contributor

@EugeneFlesselle EugeneFlesselle commented Jan 29, 2024

The Typer inserts casts for trees which only conform using gadt constraints, so that -Ycheck succeeds in later phases.
We recheck in an empty GadtState, whether the type is found to already be a subtype, in which case we do not add the cast. The issue is that isSubtype relies on the baseType cache, which might have been populated in a ctx with the narrowing gadt constraints.

https://github.com/lampepfl/dotty/blob/1716bcd9dbefbef88def848c09768a698b6b9ed9/compiler/src/dotty/tools/dotc/typer/Typer.scala#L4075-L4084

Fixes #19521

@EugeneFlesselle
Copy link
Contributor Author

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/19562/ to see the changes.

Benchmarks is based on merging with main (b20747d)

@EugeneFlesselle
Copy link
Contributor Author

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/19562/ to see the changes.

Benchmarks is based on merging with main (b20747d)

@EugeneFlesselle
Copy link
Contributor Author

EugeneFlesselle commented Jan 30, 2024

It looks like simply not caching base types when the gadtState is non empty does not really affect performance.

@EugeneFlesselle EugeneFlesselle changed the title [WIP] Only cache baseTypes when gadtState is empty Only cache base types when gadt state is empty Jan 30, 2024
@EugeneFlesselle EugeneFlesselle marked this pull request as ready for review January 30, 2024 10:17
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Feels reasonable, on balance.

@EugeneFlesselle EugeneFlesselle merged commit 51542ec into scala:main Jan 30, 2024
19 checks passed
@EugeneFlesselle EugeneFlesselle deleted the issues/i19521 branch February 6, 2024 08:13
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@scala scala deleted a comment from dottybot Mar 21, 2024
@scala scala deleted a comment from dottybot Mar 21, 2024
WojciechMazur added a commit that referenced this pull request Jul 1, 2024
)

Backports #19562 to the LTS branch.

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

Successfully merging this pull request may close these issues.

Use of GADTs and Union/Intersection types cause compiler to fail -Ycheck:all
4 participants