"exec(a, b, c)" not the same as "exec a in b, c" in nested functions

[rjordens]

BPO	21591
Nosy	@gvanrossum, @brettcannon, @birkenfeld, @terryjreedy, @ncoghlan, @vstinner, @benjaminp, @djc
Files	bug21591.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/djc'
closed_at = <Date 2014-08-10.08:21:45.634>
created_at = <Date 2014-05-28.08:47:59.370>
labels = ['interpreter-core', 'type-bug']
title = '"exec(a, b, c)" not the same as "exec a in b, c" in nested functions'
updated_at = <Date 2014-08-10.12:37:48.925>
user = 'https://bugs.python.org/rjordens'

bugs.python.org fields:

activity = <Date 2014-08-10.12:37:48.925>
actor = 'ncoghlan'
assignee = 'djc'
closed = True
closed_date = <Date 2014-08-10.08:21:45.634>
closer = 'Arfrever'
components = ['Interpreter Core']
creation = <Date 2014-05-28.08:47:59.370>
creator = 'rjordens'
dependencies = []
files = ['36061']
hgrepos = []
issue_num = 21591
keywords = ['patch']
message_count = 16.0
messages = ['219259', '219417', '223680', '223701', '223814', '223816', '224228', '224229', '224787', '224792', '225107', '225120', '225121', '225127', '225135', '225139']
nosy_count = 12.0
nosy_names = ['gvanrossum', 'brett.cannon', 'georg.brandl', 'terry.reedy', 'ncoghlan', 'vstinner', 'benjamin.peterson', 'djc', 'Neil Muller', 'Arfrever', 'python-dev', 'rjordens']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21591'
versions = ['Python 2.7']

[rjordens]

According to the documentation the "exec a in b, c" is equivalent to "exec(a, b, c)". But in the testcase below the tuple form causes a SyntaxError while the statement form works fine.

diff -r e770d8c4291c Lib/test/test_compile.py
--- a/Lib/test/test_compile.py	Tue May 27 03:30:44 2014 -0400
+++ b/Lib/test/test_compile.py	Wed May 28 02:45:31 2014 -0600
@@ -90,6 +90,22 @@
         with self.assertRaises(TypeError):
             exec("a = b + 1", g, l) in g, l
 
+    def test_nested_qualified_exec(self):
+        # Can use qualified exec in nested functions.
+        code = ["""
+def g():
+    def f():
+        if True:
+            exec "" in {}, {}
+        """, """
+def g():
+    def f():
+        if True:
+            exec("", {}, {})
+        """]
+        for c in code:
+            compile(c, "<code>", "exec")
+
     def test_exec_with_general_mapping_for_locals(self):
 
         class M:

SyntaxError: unqualified exec is not allowed in function 'f' it is a nested function (<code>, line 5)

[terryjreedy]

The exception appears to be intentional, though I do not know what a 'qualified' exec would be. But since the tuple form is intended to mimic 3.x exec, and since a reduced version of your example

c = '''
def g():
    def f():
        if True:
            exec("", {}, {})
'''
compile(c, "<code>", "exec")

runs fine in 3.4, I agree that this appears to be a 2.7 compiler bug.

[gvanrossum]

This does appear to be a bug. Please research the C code that originates the error message -- there's probably a simple logic mistake.

[NeilMuller]

Poking at the source of the error suggests the problem is in symtable.c:

The offending logic looks to be (around line 1124 in python 2.7 at revision 91767:4cef7b0ec659):

if (s->v.Exec.globals) {
   ...
}
else
{
   st->st_cur->ste_unoptimized |= OPT_BARE_EXEC;
}

since OPT_BARE_EXEC is the flag that triggers the exception.

As far as I can see, this makes no provision for the exec() case, and only avoids setting OPT_BARE_EXEC if globals is specified using the old syntax.

[djc]

I came up with a patch that shifts the compatibility hack we have for the tuple form of exec from run-time (in exec_statement()) to the CST-to-AST transformation (in ast_for_exec_stmt()). It seems to pass the tests (including the ones Robert pasted in here). Please review.

[djc]

Oh, one specific question: I'm not sure if I should free the "old" expr1 (the top-level exec value) before overwriting it with the new one.

[python-dev]

New changeset 33fb5600e9a1 by Dirkjan Ochtman in branch '2.7':
Issue bpo-21591: Handle exec backwards compatibility in the AST builder.
http://hg.python.org/cpython/rev/33fb5600e9a1

New changeset 6c47c6d2033e by Robert Jordens in branch '2.7':
Issue bpo-21591: add test for qualified exec in tuple form.
http://hg.python.org/cpython/rev/6c47c6d2033e

[djc]

Thanks to Victor Stinner for the review!

[Arfrever]

Commit 33fb5600e9a1 causes 1 test failure in test suite of py (https://pypi.python.org/pypi/py).
Test suite of py requires pytest (https://pypi.python.org/pypi/pytest)
The failing test (test_excinfo_no_python_sourcecode) requires Jinja (https://pypi.python.org/pypi/Jinja2) and is skipped otherwise.
This test also passes with Python 3.*.

Output with py 1.4.23, pytest 2.6.0 and Jinja 2.7.3:

$ python2.7 -m pytest testing/code/test_excinfo.py
==================================================================== test session starts 

====================================================================
platform linux2 -- Python 2.7.9 -- py-1.4.23 -- pytest-2.6.0
collected 71 items

testing/code/test_excinfo.py ....................F..................................................

========================================================================= FAILURES ==========================================================================
_____________________________________________________________ test_excinfo_no_python_sourcecode _____________________________________________________________

tmpdir = local('/tmp/pytest-0/test_excinfo_no_python_sourcec0')

    def test_excinfo_no_python_sourcecode(tmpdir):
        #XXX: simplified locally testable version
        tmpdir.join('test.txt').write("{{ h()}}:")
    
        jinja2 = py.test.importorskip('jinja2')
        loader = jinja2.FileSystemLoader(str(tmpdir))
        env = jinja2.Environment(loader=loader)
>       template = env.get_template('test.txt')

testing/code/test_excinfo.py:290:

---

/usr/lib64/python2.7/site-packages/jinja2/environment.py:791: in get_template
return self._load_template(name, self.make_globals(globals))
/usr/lib64/python2.7/site-packages/jinja2/environment.py:765: in _load_template
template = self.loader.load(self, name, globals)
/usr/lib64/python2.7/site-packages/jinja2/loaders.py:135: in load
globals, uptodate)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'jinja2.environment.Template'>, environment = <jinja2.environment.Environment object at 0x7f0ed6f26f10>
code = <code object <module> at 0x7f0ed6f2d930, file "/tmp/pytest-0/test_excinfo_no_python_sourcec0/test.txt", line 1>
globals = {'cycler': <class 'jinja2.utils.Cycler'>, 'dict': <function <lambda> at 0x7f0ed73c97d0>, 'joiner': <class 'jinja2.utils.Joiner'>, 'lipsum': <function generate_lorem_ipsum at 0x7f0ed7835398>, ...}
uptodate = <function uptodate at 0x7f0ed6f22ed8>

    @classmethod
    def from_code(cls, environment, code, globals, uptodate=None):
        """Creates a template object from compiled code and the globals.  This
            is used by the loaders and environment to create a template object.
            """
        namespace = {
            'environment':  environment,
            '__file__':     code.co_filename
        }
>       exec(code, namespace)
E       TypeError: exec: arg 1 must be a string, file, or code object

/usr/lib64/python2.7/site-packages/jinja2/environment.py:917: TypeError
================================================================== short test summary info ==================================================================
FAIL testing/code/test_excinfo.py::test_excinfo_no_python_sourcecode
============================================================ 1 failed, 70 passed in 1.66 seconds ============================================================

[ncoghlan]

I suspect there may also be a problem if executing pyc code generated the old way (this patch didn't bump the magic number, and doesn't really need to, so that case still needs to be handled).

Restoring the runtime check should cover it (the test can craft a suitable AST by hand rather than going through the now updated compiler)