Scan __init__ try blocks in runtime order - #3071
Conversation
scanSelfAssigns decides which self attributes a constructor is guaranteed
to have assigned before self can escape, and stops at the first statement
that reads self. For a try statement it scanned the finally block first
and treated its assignments as already made while scanning the try body.
finally runs last, so this accepted a constructor that reads an attribute
in try which is only assigned in finally:
def __init__(self):
try:
self.first = self.second + 1
finally:
self.second = 2
That compiled, and at runtime the read of self.second saw an uninitialized
field. The scan now follows execution order: the try body, then each
handler and the else block as alternative continuations, then finally.
Along the way it keeps the number of leading statements that lie before
the first self escape, so the same walk can report where the declarative
prefix of the constructor ends.
The example above is now rejected with "Attribute 'first' is not
initialized", and is the first snapshot test for that check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7526a1f797
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| finallyCrossesBoundary = scanCrossesBoundary finallyResult | ||
| assignsBeforeBoundary | ||
| | pathCrossesBoundary = guaranteedFromPaths | ||
| | finallyCrossesBoundary = finallyAssigns |
There was a problem hiding this comment.
Preserve try-path assignments when finally exposes self
When every normal try/handler path initializes an attribute and finally initializes another attribute before exposing self—for example, try: self.a = 1; finally: self.b = 2; publish(self)—finallyCrossesBoundary is true and this arm retains only finallyAssigns. It drops guaranteedFromPaths even though those assignments occurred before the finally block and therefore before the escape, causing the valid constructor to be rejected as leaving a uninitialized; preserve the preceding guaranteed assignments in this arm.
Useful? React with 👍 / 👎.
7526a1f to
caf5471
Compare
|
Closing for local review before opening; branch kept. |
scanSelfAssigns decides which self attributes a constructor is guaranteed to have assigned before self can escape. For a try statement it scanned the finally block first and counted its assignments as already made while scanning the try body, even though finally runs last. A constructor that reads an attribute inside try which is only assigned in finally therefore compiled, and the read saw an uninitialized field at runtime.
The scan now follows execution order: the try body, then each handler and the else block as alternative continuations, then finally. The example is now rejected with "Attribute 'first' is not initialized", and becomes the first snapshot test for that check.