Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

Closed
Closed
Show file tree
Hide file tree
Changes from1 commit
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
NextNext commit
Fix leaky mangling
  • Loading branch information
@JelleZijlstra
JelleZijlstra committedMay 22, 2024
commit6bb58cbf879f0b30fb2557c71fefd3d66e1a90e8
3 changes: 2 additions & 1 deletionLib/importlib/_bootstrap_external.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -473,6 +473,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.13a5 3569 (Specialize CONTAINS_OP)
# Python 3.13a6 3570 (Add __firstlineno__ class attribute)
# Python 3.14a1 3600 (Add LOAD_COMMON_CONSTANT)
# Python 3.14a1 3601 (Fix leaky name mangling in generic classes)

# Python 3.15 will start with 3700

Expand All@@ -489,7 +490,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.

MAGIC_NUMBER = (3571).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3601).to_bytes(2, 'little') + b'\r\n'

_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

Expand Down
33 changes: 33 additions & 0 deletionsLib/test/test_type_params.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -823,6 +823,39 @@ def meth[__U](self, arg: __T, arg2: __U):

self.assertEqual(Foo.Alias.__value__, (T, V))

def test_no_leaky_mangling_in_module(self):
ns = run_code("""
__before = "before"
class X[T]: pass
__after = "after"
""")
self.assertEqual(ns["__before"], "before")
self.assertEqual(ns["__after"], "after")

def test_no_leaky_mangling_in_function(self):
ns = run_code("""
def f():
class X[T]: pass
_X_foo = 2
__foo = 1
assert locals()['__foo'] == 1
return __foo
""")
self.assertEqual(ns["f"](), 1)

def test_no_leaky_mangling_in_class(self):
ns = run_code("""
class Outer:
__before = "before"
class Inner[T]:
__x = "inner"
Copy link
Member

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?

Copy link
MemberAuthor

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.

__after = "after"
""")
Outer = ns["Outer"]
self.assertEqual(Outer._Outer__before, "before")
self.assertEqual(Outer.Inner._Inner__x, "inner")
self.assertEqual(Outer._Outer__after, "after")


class TypeParamsComplexCallsTest(unittest.TestCase):
def test_defaults(self):
Expand Down
2 changes: 2 additions & 0 deletionsPython/compile.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2624,6 +2624,7 @@ compiler_class(struct compiler *c, stmt_ty s)

asdl_type_param_seq *type_params = s->v.ClassDef.type_params;
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));
Copy link
Member

@carljmcarljmMay 22, 2024•
edited
Loading

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.

JelleZijlstra reacted with thumbs up emoji
PyObject *type_params_name = PyUnicode_FromFormat("<generic parameters of %U>",
Expand DownExpand Up@@ -2701,6 +2702,7 @@ compiler_class(struct compiler *c, stmt_ty s)
s->v.ClassDef.bases,
s->v.ClassDef.keywords));
}
Py_XSETREF(c->u->u_private, old_u_private);

/* 6. apply decorators */
RETURN_IF_ERROR(compiler_apply_decorators(c, decos));
Expand Down
4 changes: 3 additions & 1 deletionPython/symtable.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1508,7 +1508,6 @@ symtable_enter_type_param_block(struct symtable *st, identifier name,
lineno, col_offset, end_lineno, end_col_offset)) {
return 0;
}
st->st_private = name;
// This is used for setting the generic base
_Py_DECLARE_STR(generic_base, ".generic_base");
if (!symtable_add_def(st, &_Py_STR(generic_base), DEF_LOCAL,
Expand DownExpand Up@@ -1673,13 +1672,15 @@ symtable_visit_stmt(struct symtable *st, stmt_ty s)
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;
Copy link
Member

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.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That would introduce the#119311 bug to non-generic classes too. However, you're right that this can be simplified. I'm about to open an alternative PR that fixes both this and#119311.

carljm reacted with thumbs up emoji
if (asdl_seq_LEN(s->v.ClassDef.type_params) > 0) {
if (!symtable_enter_type_param_block(st, s->v.ClassDef.name,
(void *)s->v.ClassDef.type_params,
false, false, s->kind,
LOCATION(s))) {
VISIT_QUIT(st, 0);
}
st->st_private = s->v.ClassDef.name;
VISIT_SEQ(st, type_param, s->v.ClassDef.type_params);
}
VISIT_SEQ(st, expr, s->v.ClassDef.bases);
Expand DownExpand Up@@ -1709,6 +1710,7 @@ symtable_visit_stmt(struct symtable *st, stmt_ty s)
if (!symtable_exit_block(st))
VISIT_QUIT(st, 0);
}
st->st_private = old_st_private;
break;
}
case TypeAlias_kind: {
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp