gh-119395: Fix leaky mangling after generic classes#119399
gh-119395: Fix leaky mangling after generic classes#119399JelleZijlstra wants to merge 4 commits intopython:mainfrom
Conversation
| class Outer: | ||
| __before = "before" | ||
| class Inner[T]: | ||
| __x = "inner" |
There was a problem hiding this comment.
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.
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
| int is_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.
The save-restore pattern is not needed or used in compiler_class_body, because there c->u->u_private is set after compiler_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, because compiler_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 the compiler_enter_scope call that seems to require mangling?
If we move this line down to after compiler_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 handling u_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.
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 have st_private set during lines 1686 to 1691, then that would imply that your current version is problematic as well.
|
#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.