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 TyperState assertion failures #12827

Merged
merged 2 commits into from Jun 15, 2021

Conversation

smarter
Copy link
Member

@smarter smarter commented Jun 15, 2021

I nominate this PR for a backport to 3.0.1 since it fixes a regression introduced in 3.0.1-RC1 by #12519: that PR added some assertions which are now failing, this PR does not remove the assertions but fixes the underlying issues.

The TyperState of `FunProto#protoCtx` might already be committed by the
time we call `FunProto#typedArgs` (this can happen for example with
the nested TyperState in a `tryEither`), in that case we can trigger
assertion errors if we touch that TyperState again. Fixed by using the
nearest uncommited ancestor of the capture TyperState instead.
Flushing the reporter can force messages (apparently because of
`HideNonSensicalMessages#isHidden`) which can affect the TyperState
we're in the process of committing, so make sure that flushing happens
before committing to not trigger any assertion.

Fixes scala#12736
@smarter smarter added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jun 15, 2021
@smarter smarter requested a review from odersky June 15, 2021 12:35
@smarter smarter linked an issue Jun 15, 2021 that may be closed by this pull request
@odersky odersky merged commit 16e07f4 into scala:master Jun 15, 2021
@odersky odersky deleted the skip-already-committed branch June 15, 2021 16:50
@smarter smarter added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Jun 15, 2021
@smarter smarter added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Jun 16, 2021
smarter added a commit to dotty-staging/dotty that referenced this pull request Aug 28, 2021
An already-committed TyperState might be committed again when errors are
flushed (cf scala#12827, scala#13150) and `commit()` calls `gc()`. This operation
could crash before this commit because we attempted to instantiate type
variables no longer owned by the TyperState. We fix this by clearing
`ownedVars` when committing a TyperState (because after committing it no
longer owns any type variable).

Fixes scala#13407.
olsdavis pushed a commit to olsdavis/dotty that referenced this pull request Apr 4, 2022
An already-committed TyperState might be committed again when errors are
flushed (cf scala#12827, scala#13150) and `commit()` calls `gc()`. This operation
could crash before this commit because we attempted to instantiate type
variables no longer owned by the TyperState. We fix this by clearing
`ownedVars` when committing a TyperState (because after committing it no
longer owns any type variable).

Fixes scala#13407.
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failure when committing TyperState
3 participants