-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Sema: Undo changes in chronological order in SolverTrail::undo() #77174
base: main
Are you sure you want to change the base?
Conversation
These functions are never called while undoing a change; so we want to assert if we have an active undo instead of silently not recording the change.
void ConstraintGraphNode::reintroduceToInference(Constraint *constraint) { | ||
retractFromInference(constraint); | ||
introduceToInference(constraint); | ||
void ConstraintGraphNode::retractFromInference(Type fixedType) { |
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.
Looks like the logic in retractFromInference(Type)
and introduceToInference
could be unified into a method i.e.
updateAfterBind` that would accept a callback that either introduces or retracts constraints...
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.
Good idea.
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! I feel like dealing with constraints after binding a type variable is an overkill, we might get away with keeping the set as is but using simplifyType during BindingSet construction because all of the logic to compute flags (i.e. pontential incomplete, literal, delayed etc.) moved there.
This was a loose end from my SolverScope refactoring.