Skip to content

bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported objects #26677

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

Merged
merged 2 commits into from
Jun 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions Include/internal/pycore_symtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ extern PyTypeObject PySTEntry_Type;

#define PySTEntry_Check(op) Py_IS_TYPE(op, &PySTEntry_Type)

extern long _PyST_GetSymbol(PySTEntryObject *, PyObject *);
extern int _PyST_GetScope(PySTEntryObject *, PyObject *);

extern struct symtable* _PySymtable_Build(
Expand Down
1 change: 1 addition & 0 deletions Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.11a1 3455 (add MAKE_CELL bpo-43693)
# Python 3.11a1 3456 (interleave cell args bpo-43693)
# Python 3.11a1 3457 (Change localsplus to a bytes object bpo-43693)
# Python 3.11a1 3458 (imported objects now don't use LOAD_METHOD/CALL_METHOD)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the actual number be bumped below? I don't think it matters much in this case, but for the sake of consistency/clarity for the next bump, it might be good to fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, probably got away during rebase. Will fix


#
# MAGIC must change whenever the bytecode emitted by the compiler may no
Expand Down
36 changes: 36 additions & 0 deletions Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import _ast
import tempfile
import types
import textwrap
from test import support
from test.support import script_helper
from test.support.os_helper import FakePath
Expand Down Expand Up @@ -791,6 +792,41 @@ def or_false(x):
self.assertIn('LOAD_', opcodes[0].opname)
self.assertEqual('RETURN_VALUE', opcodes[1].opname)

def test_imported_load_method(self):
sources = [
"""\
import os
def foo():
return os.uname()
""",
"""\
import os as operating_system
def foo():
return operating_system.uname()
""",
"""\
from os import path
def foo(x):
return path.join(x)
""",
"""\
from os import path as os_path
def foo(x):
return os_path.join(x)
"""
]
for source in sources:
namespace = {}
exec(textwrap.dedent(source), namespace)
func = namespace['foo']
with self.subTest(func=func.__name__):
opcodes = list(dis.get_instructions(func))
instructions = [opcode.opname for opcode in opcodes]
self.assertNotIn('LOAD_METHOD', instructions)
self.assertNotIn('CALL_METHOD', instructions)
self.assertIn('LOAD_ATTR', instructions)
self.assertIn('CALL_FUNCTION', instructions)

def test_lineno_procedure_call(self):
def call():
(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Directly imported objects and modules (through import and from import
statements) don't generate ``LOAD_METHOD``/``CALL_METHOD`` for directly
accessed objects on their namespace. They now use the regular
``LOAD_ATTR``/``CALL_FUNCTION``.
2 changes: 1 addition & 1 deletion Programs/test_frozenmain.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -4278,6 +4278,23 @@ check_index(struct compiler *c, expr_ty e, expr_ty s)
}
}

static int
is_import_originated(struct compiler *c, expr_ty e)
{
/* Check whether the global scope has an import named
e, if it is a Name object. For not traversing all the
scope stack every time this function is called, it will
only check the global scope to determine whether something
is imported or not. */

if (e->kind != Name_kind) {
return 0;
}

long flags = _PyST_GetSymbol(c->c_st->st_top, e->v.Name.id);
return flags & DEF_IMPORT;
}

// Return 1 if the method call was optimized, -1 if not, and 0 on error.
static int
maybe_optimize_method_call(struct compiler *c, expr_ty e)
Expand All @@ -4291,6 +4308,12 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e)
if (meth->kind != Attribute_kind || meth->v.Attribute.ctx != Load) {
return -1;
}

/* Check that the base object is not something that is imported */
if (is_import_originated(c, meth->v.Attribute.value)) {
return -1;
}

/* Check that there aren't too many arguments */
argsl = asdl_seq_LEN(args);
kwdsl = asdl_seq_LEN(kwds);
Expand Down
Loading