-
Notifications
You must be signed in to change notification settings - Fork 11
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
Clean up the way we read recursive types. #291
Conversation
For `RecType`/`RecThis`, use the infrastructure for enclosing binders in the `localCtx`, like we do for `LambdaType`s. For the `THIS` references to the enclosing refinement class, we now create the magic refinement class symbols along with all the other symbols, and then we specially treat `REFINEDtpt` in `readType` position to fetch that symbol.
@@ -1185,6 +1183,15 @@ private[tasties] class TreeUnpickler( | |||
val scrutinee = readType | |||
val cases: List[MatchTypeCase] = reader.until(end)(readMatchTypeCase) | |||
MatchType(upperBound, scrutinee, cases) | |||
case REFINEDtpt => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the test case that exercises this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or at least what's some source code to reproduce tasty with this in type position?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any refinement type tree written in the source code that contains a recursive this
reference. We already have tests for that. The this
references are THIS
nodes containing a SHAREDtype
that points to the REFINEDtpt
. It's absurd, IMO, but that's how it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that makes sense
Previously, a different `sharedTypesCache` was created for every instance of `TreeUnpickler`... which means at every `fork`. The cache was therefore not used in many cases where it should be. Now, we make sure to preserve caches across instances of `TreeUnpickler`.
See the comment for the rationale. This was found in the wild (in the published artifacts of `perspective` in particular) but it is not clear how to reproduce it.
I added two more commits that support more recursive stuff. |
throw TastyFormatException(s"Unexpected member $otherDef in refinement type") | ||
} | ||
RefinedTypeTree(parent, refinements, cls)(spn) | ||
/* It is possible to find SHAREDterm's referencing REFINEDtpt's. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the point of SHAREDterm and SHAREDtype is that its literally a shared reference that the parser should reuse, and never re-parse - aka tasty ast should not have observable mutability :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might think so, but even dotc disagrees. It truly caches Types, but it re parses Trees and TypeTrees. And in this case, it will in fact re parse trees defining the same symbols, creating something wrong in the compiler. 🤷♂️
For
RecType
/RecThis
, use the infrastructure for enclosing binders in thelocalCtx
, like we do forLambdaType
s.For the
THIS
references to the enclosing refinement class, we now create the magic refinement class symbols along with all the other symbols, and then we specially treatREFINEDtpt
inreadType
position to fetch that symbol.