-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
bpo-47084: Clear cached representations on finalization #32032
Conversation
('off', 'import __hello__'), | ||
('on', 'import __hello__'), | ||
) | ||
for flag, stmt in tests: |
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.
Can you please use with self.subTest(flag=flag, stmt=stmt):
? Otherwise, it will be more complicated to debug if a test fails. With that, you can omit f"{xopt}, stmt='{stmt}' -> {out}")
below.
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.
Didn't know about TestCase.subTest()
, thanks.
|
||
|
||
void | ||
_PyStaticUnicode_Dealloc(PyObject *op) |
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.
Would it make sense to share code between this function and unicode_dealloc()? Maybe add an unicode_dealloc_common() function?
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, there is the duplication of the PyObect_Free()
calls, but not really to the level of what I would consider "in common". I did, however, refactor out the duplication of the 1-character singletons and frozen strings deallocation into a standalone static function.
What started out as different behavior for each type ended up being identical after edge cases were discovered.
Merged, thanks! Well spotted! |
https://bugs.python.org/issue47084