-
Notifications
You must be signed in to change notification settings - Fork 32
Fix constraint cache and unexpected generalizations in BbML #305
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
The head ref may contain hidden characters: "patch\u{1F6DD}"
Conversation
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.
Why do we cache the Basic type (and not, say, the normalized one)? Can you explain what went wrong in the previous version?
I think the real wrong place is here: abstract class TypeExt extends Type:
// ...
override def equals(that: Any): Bool =
toBasic === that If the RHS is not a basic type, we cannot retrieve the constraint from the cache, leading to a stack overflow. Here is the hash set dumped when we solve the constraint HashSet(
(Conj(Inter(None),Union(None,List()),List((InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),true))),NegType(ComposedType(ClassLikeType(class:Cons‹1054›,List(Wildcard(hkmc2.bbml.Bot$@14189e29,NegType(hkmc2.bbml.Bot$@14189e29)), Wildcard(hkmc2.bbml.Bot$@14189e29,NegType(hkmc2.bbml.Bot$@14189e29)))),NegType(ComposedType(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))),ClassLikeType(class:Nil‹1052›,List()),true)),false))),
(InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),InfVar(1,103,hkmc2.bbml.VarState@662f0df5,false)),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))),NegType(ComposedType(ClassLikeType(class:Cons‹1054›,List(Wildcard(hkmc2.bbml.Bot$@14189e29,NegType(hkmc2.bbml.Bot$@14189e29)), Wildcard(hkmc2.bbml.Bot$@14189e29,NegType(hkmc2.bbml.Bot$@14189e29)))),NegType(ComposedType(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))),ClassLikeType(class:Nil‹1052›,List()),true)),false))),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))),NegType(ComposedType(NegType(hkmc2.bbml.Bot$@14189e29),NegType(InfVar(1,103,hkmc2.bbml.VarState@662f0df5,false)),false))),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),true))),InfVar(1,103,hkmc2.bbml.VarState@662f0df5,false)),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))),InfVar(1,103,hkmc2.bbml.VarState@662f0df5,false)),
(InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List()))),
(InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List()))),
(InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List()))),
(InfVar(1,97,hkmc2.bbml.VarState@6e9f9e57,false),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List()))),
(ComposedType(Disj(List(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))))),Disj(List(Conj(Inter(None),Union(None,List()),List()))),false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)),
(ComposedType(Disj(List(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))))),Disj(List(Conj(Inter(None),Union(None,List()),List()))),false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)),
(Conj(Inter(None),Union(None,List()),List((InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false),true))),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)),
(ClassLikeType(class:Cons‹1054›,List(InfVar(1,98,hkmc2.bbml.VarState@14f0f18b,false), InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false))),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List()))),
(ClassLikeType(class:Cons‹1054›,List(InfVar(1,98,hkmc2.bbml.VarState@14f0f18b,false), InfVar(1,99,hkmc2.bbml.VarState@62b2702b,false))),NegType(Conj(Inter(Some(ClassLikeType(class:Cons‹1054›,List(Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))), Wildcard(Disj(List()),Disj(List(Conj(Inter(None),Union(None,List()),List())))))))),Union(None,List(ClassLikeType(class:Cons‹1054›,List(Wildcard(InfVar(1,105,hkmc2.bbml.VarState@922ccdb,false),InfVar(1,106,hkmc2.bbml.VarState@4dcb19ef,false)), Wildcard(InfVar(1,107,hkmc2.bbml.VarState@606a2aed,false),InfVar(1,108,hkmc2.bbml.VarState@3349dd58,false)))), ClassLikeType(class:Nil‹1052›,List()))),List())))
) I think we can cache normalized types, but we still need to remove this overridden |
Oh yeah that |
v.state.upperBounds.foreach(ub => constrainImpl(bd, ub)) | ||
val c = bd.toDnf | ||
log(s"New bound: ${v.showDbg} :> ${c.showDbg}") | ||
cctx.nest(c -> v) givenIn: |
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 just noticed you're using a purely functional cache. Doing so has the wrong complexity (exponential instead of quadratic). Are we doing this to mirror the formally described system in the paper?
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.
Yes, we use a purely functional cache in the paper. But we can also adopt a mutable one in the implementation (actually, there is already a TODO comment above the CCtx
class for 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.
Ok, can do it in another PR if necessary.
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.
LGTM. The type pretty-printing is also much better, now.
Bugs found by @auht.
.toBasic
. Also improved type printing in error messages.