Skip to content

[WIP] make @compile_fun correctly recurse through imported names, to try to fix #52 #53

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[WIP] make @compile_fun correctly recurse through imported names, to try to fix #52 #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44a0251
Work in progress to try to fix #52
Mar 23, 2020
6ae2776
Improved and fixed test
Mar 23, 2020
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
100 changes: 61 additions & 39 deletions makefun/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,8 @@ def apply_compile_fun(target):
def compile_fun_manually(target,
recurse=True, # type: Union[bool, Callable]
except_names=(), # type: Iterable[str]
_evaldict=None # type: Union[bool, Dict]
_evaldict=None, # type: Union[bool, Dict]
_nested_call=False # type: bool
):
"""

Expand All @@ -1044,12 +1045,27 @@ def compile_fun_manually(target,
if not isinstance(target, FunctionType):
raise UnsupportedForCompilation("Only functions can be compiled by this decorator")

if _evaldict is None or _evaldict is True:
if _evaldict is True:
frame = _get_callerframe(offset=1)
else:
frame = _get_callerframe()
_evaldict, _ = extract_module_and_evaldict(frame)
if not _nested_call:
# top call - grab the eval dict from the frame
if _evaldict is None or _evaldict is True:
if _evaldict is True:
frame = _get_callerframe(offset=1)
else:
frame = _get_callerframe()
_evaldict, module_name = extract_module_and_evaldict(frame)

if target.__module__ != module_name:
if hasattr(target, '__globals__'):
_evaldict = copy(target.__globals__)
# else:
# advanced - provided by user
else:
# nested: TODO should we propagate the eval dict ? How ? what about locals ?
# Note: this should be done bu
if hasattr(target, '__globals__'):
_evaldict = copy(_evaldict)
_evaldict.update(target.__globals__)
# TODO if value is a class, no __globals__ exist (they do on the functions only)

# first make sure that source code is available for compilation
try:
Expand All @@ -1070,38 +1086,44 @@ def compile_fun_manually(target,
func_closure = target.func_closure
func_code = target.func_code

# Does not work: if `self.i` is used in the code, `i` will appear here
# if func_code is not None:
# for name in func_code.co_names:
# try:
# eval(name, _evaldict)
# except NameError:
# raise UndefinedSymbolError("Symbol `%s` does not seem to be defined yet. Make sure you apply "
# "`compile_fun` *after* all required symbols have been defined." % name)

if recurse and func_closure is not None:
# recurse-compile
for name, cell in zip(func_code.co_freevars, func_closure):
if name in except_names:
continue
if name not in _evaldict:
raise UndefinedSymbolError("Symbol %s does not seem to be defined yet. Make sure you apply "
"`compile_fun` *after* all required symbols have been defined." % name)
try:
value = cell.cell_contents
except ValueError:
# empty cell
continue
else:
# non-empty cell
try:
# note : not sure the compilation will be made in the appropriate order of dependencies...
# if not, users will have to do it manually
_evaldict[name] = compile_fun_manually(value,
recurse=recurse, except_names=except_names,
_evaldict=_evaldict)
except (UnsupportedForCompilation, SourceUnavailable):
pass
if recurse:
# recurse-compile used symbols
# already compiled since part of the source
# - co_varnames: everything created locally, including args -
# - co_cellvars: local variables that are referenced by nested functions
# to compile:
# - co_freevars: non-local variables that are referenced by nested functions
# - co_names: global variables used by the bytecode, local names in the class scope, attribute names, names of
# imported modules, etc. see https://github.com/python/cpython/pull/2743
if func_code is not None:
for names, raise_if_unknown in ((func_code.co_freevars, True),
(func_code.co_names, False) # we can't do this now: this triggers some strange bugs with the docstrings and other things. see deopatch tests.
):
for name in names:
if name in except_names:
continue
try:
value = _evaldict[name]
except KeyError:
if raise_if_unknown:
raise UndefinedSymbolError("Symbol %s does not seem to be defined yet. Make sure you apply "
"`compile_fun` *after* all required symbols have been defined."
% name)
else:
# names in co_names can be attribute names, e.g. 'i' if self.i is used. So unknown will
# happen.
continue
else:
try:
#

# note : not sure the compilation will be made in the appropriate order of dependencies...
# if not, users will have to do it manually
_evaldict[name] = compile_fun_manually(value,
recurse=recurse, except_names=except_names,
_evaldict=_evaldict, _nested_call=True)
except (UnsupportedForCompilation, SourceUnavailable):
pass

# now compile from sources
lines = dedent(lines)
Expand Down
14 changes: 9 additions & 5 deletions makefun/tests/test_compile_deco.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ def bar(a, b):
def test_compilefun_co_names():
"""Test that today we do not compile imported names."""

# TODO one day it would be great to selectively recurse through such imported names. Unfortunately,
# this comes with *many* side effects including compilation order, appropriate propagation or
# non-propagation of globals(), locals()
# See https://github.com/smarie/python-makefun/issues/52

# TODO this test is not sufficient, there are things happening in more complex settings that are not happening here.
# see https://github.com/smarie/python-decopatch/pull/18

@compile_fun
def foo():
# TODO one day it would be great to selectively recurse through such imported names. Unfortunately,
# this comes with *many* side effects including compilation order, appropriate propagation or
# non-propagation of globals(), locals()
# See https://github.com/smarie/python-makefun/issues/52
assert not is_compiled(dedent)
assert is_compiled(dedent)
return dedent(" hoho")

res = foo()
Expand Down