Skip to content

baseType caching is broken because the cached type can contain uninstantiated type variables #5333

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

Closed
smarter opened this issue Oct 29, 2018 · 2 comments

Comments

@smarter
Copy link
Member

smarter commented Oct 29, 2018

In ClassDenotation#baseTypeOf, we have: https://github.com/lampepfl/dotty/blob/cedef0d317e39ee114a5ec0409604613837743a2/compiler/src/dotty/tools/dotc/core/SymDenotations.scala#L1697-L1707

Notice that record(tp, baseTp) is always called, even if tp contains uninstantiated type variables which could later be instantiated in a way that changes the base type. I haven't been able to make a test case that shows how this is broken in master, but I've hit this in a branch where I'm doing unrelated changes. I was able to work around the issue by doing:

if (isFullyDefined(tp, ForceDegree.none))
  record(tp, baseTp)
else
  btrCache.remove(tp) // Remove the "NoPrefix" sentinel value

but that causes many tests to fail with a cyclic error.

@odersky WDYT ?

@smarter
Copy link
Member Author

smarter commented Oct 29, 2018

Oh, I had forgotten about Type#isProvisional, that seems to be the correct thing to check before adding values to the cache!

@odersky
Copy link
Contributor

odersky commented Oct 30, 2018

Yes, exactly.

odersky added a commit that referenced this issue Oct 31, 2018
Fix #5333: Invalid entries in the baseType cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants