Skip to content

Fix #1812, Symbols.mapSymbols shouldn't replace denotations #1832

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

Merged
merged 3 commits into from
Dec 20, 2016

Conversation

DarkDimius
Copy link
Contributor

@DarkDimius DarkDimius commented Dec 19, 2016

It will use lazy types instead. The current version transforms a type, with
a context that has denotations that may be forcefully replaced by
mapSymbols. Types created during this transformation may cache denots, that
are-to-be replaced. This is very problematic as this method is called from
TreeTypeMap.withMappedSyms in a fixed-point cycle, creating new symbols on
every iteration. Those cached denotations could make types keep symbols
from previous iterations indefinitely.

The changed version does not transform the types eagerly, and instead makes
them lazy. Assuming there are no cycles, this should ensure correct
ordering. Unfortunatelly, at this point in the compiler we basically always
touch everything, and we can't even transform the info of denotation
without this denotations info. We basically have a chicked&egg problem
here. To solve it, I use the same trick as used by other lazy types by
assigning an approximation of future type first. This allows to pass the
tests and makes dotty more robust, but I suspect this isn't a complete fix
and new similar bugs may arrive.

It will use lazy types instead.
The current version transforms a type, with a context that has denotations
that may be forcefully replaced by mapSymbols. Types created during this
transformation may cache denots, that are-to-be replaced.
This is very problematic as this method is called from TreeTypeMap.withMappedSyms
in a fixed-point cycle, creating new symbols on every iteration. Those cached denotations
could make types keep symbols from previous iterations indefinitely.

The changed version does not transform the types eagerly, and instead makes them lazy.
Assuming there are no cycles, this should ensure correct ordering.
Unfortunatelly, at this point in the compiler we basically always touch everything,
and we can't even transform the info of denotation without this denotations info.
We basically have a chicked&egg problem here. To solve it, I use the same trick as used by
other lazy types by assigning an approximation of future type first.
This allows to pass the tests and makes dotty more robust, but I suspect this isn't a complete
fix and new similar bugs may arrive.
@smarter
Copy link
Member

smarter commented Dec 19, 2016

Could you add the testcase from #1812 to verify that the fix work?

…rmation.

Reasoning similar to one in the previous commit also applies to annotations.
@DarkDimius
Copy link
Contributor Author

@odersky please review.

@odersky
Copy link
Contributor

odersky commented Dec 20, 2016

LGTM 👍

@odersky odersky merged commit 098c50a into scala:master Dec 20, 2016
@allanrenucci allanrenucci deleted the fix-1810 branch December 14, 2017 19: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.

4 participants