-
Notifications
You must be signed in to change notification settings - Fork 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
Fix #21619: Refactor NotNullInfo to record every reference which is retracted once. #21624
base: main
Are you sure you want to change the base?
Conversation
I think this is the right idea but reasoning about three sets is hard and it's hard to convince oneself that the implementation is correct for all cases. I wonder if a simpler implementation would be possible with only two sets. Let's still call them asserted and retracted. We lift the invariant that asserted & retracted is empty, so it's possible for one variable to be in both. asserted is the set that we know is non-null at the very end of evaluating the expression (if no exception happens). retracted is the set that is retracted anywhere within the expression, even if it is later asserted. When we assert a variable, we add it to asserted but do not remove it from retracted. When we retract a variable, we add it to retracted and remove it from asserted. If we evaluate the expression to the very end, if a variable is in both asserted and retracted, we consider it asserted. When we're at a control flow point where the expression may not have executed to the end (because it threw an exception), we just clear the asserted set and keep only the retracted set. At control flow merges, we intersect asserted and union retracted. For a sequential composition (a1,r1) ; (a2,r2), we get ((a1-r2) | a2, r1|r2). Do you think this would work or do you see any flaw in it? I think maybe what I've described above are the current sets asserted and onceRetracted in this pull request. I guess the question is, if we have asserted and onceRetracted, can we get rid of retracted? (Then eventually we could rename onceRetracted to just retracted, to make the name shorter and simpler.) |
In the above proposal:
|
This rule looks reasonable to me. I will think about more examples to double-check this. |
TODO: some other improvements I want for flow typing but haven't been implemented yet:
|
Fix #21619
This PR improves the flow typing with try block.
Since we don't know at which point an exception is thrown in the body, we have to collect any reference that is once retracted.