-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Rework MatchType recursion in collectParts #19867
Conversation
This patch fixes a recursion situation in collectParts, by reimplementing a previous fix. Recursion is already attempted to be cutoff with `partSeen`, and `handleRecursion` is also used to prevent any unhandled recursion situations (like the one fixed here) from crashing the compiler. For context, AppliedType aren't hash-consed (i.e. the flyweight pattern) which means that every time you apply the same type arguments to the same type constructor you get a fresh AppliedType. Using i18171 as an example, the sequence of events where this matters is: 0. When typing BAZ, so much earlier than the collectParts call, because the MatchType on the rhs of BAZ reduces at definition site, the RHS is reduced to the `DFVal[BAZREC[T]]`, which means that BAZ's info is a TypeAlias rather than a MatchAlias, meaning it can dealias. 1. `BAZREC[Any]` is extracted by MatchType.InDisguise, which applies the Any to return a fresh MatchType 2. `Tuple.Map[Any, BAZ]` is also extracted by MatchType.InDisguise, which returns its own fresh MatchType 3. `BAZ[h]` dealiases to a fresh `DFVal[BAZREC[h]]` instance (see step 0) 4. `BAZREC[h]` is extracted by MatchType.InDisguise, repeating the cycle The reason that the cases with MatchType.InDisguise and MatchType were introduced is i17395. Looking back, it seems the only need is to capture any parts that are in the reduction of an applied MatchType. With this patch applied in the case of i18171, this now cuts off quickly, as `BAZREC[Any]` doesn't reduce to anything. In the case of i17395, `ExtractPart[ValuePartHolder]` reduces to `Value`, so `Value` is successfully recorded as a part.
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'm not sure what you meant about the AppliedTypes not being hash-consed, we have the same problem with recursive match types in collectParts either way right ?
Otherwise looks good to me
We do have the problem either way. I was just providing it as context, for why we've got the problem, despite |
But AppliedTypes are hash-consed, so there must be something else going on: object AppliedType {
def apply(tycon: Type, args: List[Type])(using Context): AppliedType = {
assertUnerased()
ctx.base.uniqueAppliedTypes.enterIfNew(tycon, args)
}
} |
Yes, it's the type constructor that's different, because in it is a fresh LazyRef. |
This patch fixes a recursion situation in collectParts, by
reimplementing a previous fix. Recursion is already attempted to be
cutoff with
partSeen
, andhandleRecursion
is also used to preventany unhandled recursion situations (like the one fixed here) from
crashing the compiler.
For context, AppliedType aren't hash-consed (i.e. the flyweight pattern)
which means that every time you apply the same type arguments to the
same type constructor you get a fresh AppliedType. Using i18171 as an
example, the sequence of events where this matters is:
the MatchType on the rhs of BAZ reduces at definition site, the RHS
is reduced to the
DFVal[BAZREC[T]]
, which means that BAZ's info isa TypeAlias rather than a MatchAlias, meaning it can dealias.
BAZREC[Any]
is extracted by MatchType.InDisguise, which applies theAny to return a fresh MatchType
Tuple.Map[Any, BAZ]
is also extracted by MatchType.InDisguise,which returns its own fresh MatchType
BAZ[h]
dealiases to a freshDFVal[BAZREC[h]]
instance (see step 0)BAZREC[h]
is extracted by MatchType.InDisguise, repeating the cycleThe reason that the cases with MatchType.InDisguise and MatchType were
introduced is i17395. Looking back, it seems the only need is to
capture any parts that are in the reduction of an applied MatchType.
With this patch applied in the case of i18171, this now cuts off
quickly, as
BAZREC[Any]
doesn't reduce to anything. In the case ofi17395,
ExtractPart[ValuePartHolder]
reduces toValue
, soValue
issuccessfully recorded as a part.
Fixes #19857
Fixes #18171