-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-123958: move assert optimization from codegen to ast_opt #124143
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
Conversation
if (OPTIMIZATION_LEVEL(c)) { | ||
return SUCCESS; | ||
} | ||
assert(!OPTIMIZATION_LEVEL(c)); |
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.
Left this here to convince ourselves of correctness. I'll remove it before committing.
if (state->optimize) { | ||
make_pass(node_); |
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.
Technically speaking, this is not the removal of the assert
statement; it's replacement of assert
with pass
statement. Is it possible to completely remove the assert
node? Or will it break some existing code?
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.
Pass becomes a NOP, which gets removed later. This is how we get rid of code. However, I did forget to wipe out the location information (a NOP is not removed if it's the only instruction from its line in the source code).
if (optimize): | ||
co = compile(source, '?', mode, optimize=optimize) | ||
co_expected = compile('pass', '?', mode, optimize=optimize) | ||
self.assertEqual(list(co.co_lines()), list(co_expected.co_lines())) |
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.
This looks like we no longer raise an error in optimized mode if there is an invalid await. That seems wrong and doesn't align with what we decided on in #121637.
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.
If we want to stick with this then we probably can't optimise the assertions in ast_opt.
Co-authored-by: Jelle Zijlstra <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.