Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-119395: Fix leaky mangling after generic classes#119399
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| class Outer: | ||
| __before = "before" | ||
| class Inner[T]: | ||
| __x = "inner" |
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.
Is it a good idea to test__before = "class"; __after = "class" names in the class scope here? Or do we have them tested?
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.
You mean in the inner class? That wouldn't really be relevant here since there is nothing else interesting going on in the inner class body.
Python/compile.c Outdated
| intis_generic=asdl_seq_LEN(type_params)>0; | ||
| PyObject*old_u_private=Py_XNewRef(c->u->u_private); | ||
| if (is_generic) { | ||
| Py_XSETREF(c->u->u_private,Py_NewRef(s->v.ClassDef.name)); |
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.
The save-restore pattern is not needed or used incompiler_class_body, because therec->u->u_private is set aftercompiler_enter_scope, so it is set only on the inner compiler unit, not the outer one.
Setting it on the outer compiler unit works here, becausecompiler_enter_scope copies it to inner scopes. But is it required to set it on the outer unit? I don't see anything happening between here and thecompiler_enter_scope call that seems to require mangling?
If we move this line down to aftercompiler_enter_scope, does that work and fix the bug, without requiring save/restore? If so, that seems simpler and more consistent with the intended/existing pattern for handlingu_private.
Python/symtable.c Outdated
| VISIT_QUIT(st,0); | ||
| if (s->v.ClassDef.decorator_list) | ||
| VISIT_SEQ(st,expr,s->v.ClassDef.decorator_list); | ||
| PyObject*old_st_private=st->st_private; |
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.
I don't care too much either way, but I think it would be equivalent and simpler to move the existing pair of lines
tmp = st->st_private;st->st_private = s->v.ClassDef.name;up to right here. We want to unconditionally set it to the same thing either way.
If it's a problem to havest_private set during lines 1686 to 1691, then that would imply that your current version is problematic as well.
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.
JelleZijlstra commentedMay 23, 2024
#119464 supersedes this PR, but keeping this one open for now since we may want to apply this change in 3.12. |
Uh oh!
There was an error while loading.Please reload this page.