[Python-checkins] cpython: evaluate positional defaults before keyword-only defaults (closes #16967)

benjamin.peterson python-checkins at python.org
Sun Feb 10 15:32:37 CET 2013


http://hg.python.org/cpython/rev/d296cf1600a8
changeset:   82133:d296cf1600a8
parent:      82128:029011429f80
user:        Benjamin Peterson <benjamin at python.org>
date:        Sun Feb 10 09:29:59 2013 -0500
summary:
  evaluate positional defaults before keyword-only defaults (closes #16967)

files:
  Doc/reference/compound_stmts.rst |   17 +-
  Lib/importlib/_bootstrap.py      |    4 +-
  Lib/test/test_keywordonlyarg.py  |    8 +
  Misc/NEWS                        |    3 +
  Python/ceval.c                   |   34 +-
  Python/compile.c                 |    4 +-
  Python/importlib.h               |  238 +++++++++---------
  7 files changed, 161 insertions(+), 147 deletions(-)


diff --git a/Doc/reference/compound_stmts.rst b/Doc/reference/compound_stmts.rst
--- a/Doc/reference/compound_stmts.rst
+++ b/Doc/reference/compound_stmts.rst
@@ -493,14 +493,15 @@
 value, all following parameters up until the "``*``" must also have a default
 value --- this is a syntactic restriction that is not expressed by the grammar.
 
-**Default parameter values are evaluated when the function definition is
-executed.** This means that the expression is evaluated once, when the function
-is defined, and that the same "pre-computed" value is used for each call.  This
-is especially important to understand when a default parameter is a mutable
-object, such as a list or a dictionary: if the function modifies the object
-(e.g. by appending an item to a list), the default value is in effect modified.
-This is generally not what was intended.  A way around this is to use ``None``
-as the default, and explicitly test for it in the body of the function, e.g.::
+**Default parameter values are evaluated from left to right when the function
+definition is executed.** This means that the expression is evaluated once, when
+the function is defined, and that the same "pre-computed" value is used for each
+call.  This is especially important to understand when a default parameter is a
+mutable object, such as a list or a dictionary: if the function modifies the
+object (e.g. by appending an item to a list), the default value is in effect
+modified.  This is generally not what was intended.  A way around this is to use
+``None`` as the default, and explicitly test for it in the body of the function,
+e.g.::
 
    def whats_on_the_telly(penguin=None):
        if penguin is None:
diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -396,13 +396,15 @@
                      3210 (added size modulo 2**32 to the pyc header)
     Python 3.3a1  3220 (changed PEP 380 implementation)
     Python 3.3a4  3230 (revert changes to implicit __class__ closure)
+    Python 3.4a1  3240 (evaluate positional default arguments before
+                       keyword-only defaults)
 
 MAGIC must change whenever the bytecode emitted by the compiler may no
 longer be understood by older implementations of the eval loop (usually
 due to the addition of new opcodes).
 
 """
-_RAW_MAGIC_NUMBER = 3230 | ord('\r') << 16 | ord('\n') << 24
+_RAW_MAGIC_NUMBER = 3240 | ord('\r') << 16 | ord('\n') << 24
 _MAGIC_BYTES = bytes(_RAW_MAGIC_NUMBER >> n & 0xff for n in range(0, 25, 8))
 
 _PYCACHE = '__pycache__'
diff --git a/Lib/test/test_keywordonlyarg.py b/Lib/test/test_keywordonlyarg.py
--- a/Lib/test/test_keywordonlyarg.py
+++ b/Lib/test/test_keywordonlyarg.py
@@ -176,6 +176,14 @@
                 return __a
         self.assertEqual(X().f(), 42)
 
+    def test_default_evaluation_order(self):
+        # See issue 16967
+        a = 42
+        with self.assertRaises(NameError) as err:
+            def f(v=a, x=b, *, y=c, z=d):
+                pass
+        self.assertEqual(str(err.exception), "global name 'b' is not defined")
+
 def test_main():
     run_unittest(KeywordOnlyArgTestCase)
 
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
 Core and Builtins
 -----------------
 
+- Issue #16967: In function definition, evaluate positional defaults before
+  keyword-only defaults.
+
 - Issue #17173: Remove uses of locale-dependent C functions (isalpha() etc.)
   in the interpreter.
 
diff --git a/Python/ceval.c b/Python/ceval.c
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -2901,23 +2901,6 @@
             }
 
             /* XXX Maybe this should be a separate opcode? */
-            if (posdefaults > 0) {
-                PyObject *defs = PyTuple_New(posdefaults);
-                if (defs == NULL) {
-                    Py_DECREF(func);
-                    goto error;
-                }
-                while (--posdefaults >= 0)
-                    PyTuple_SET_ITEM(defs, posdefaults, POP());
-                if (PyFunction_SetDefaults(func, defs) != 0) {
-                    /* Can't happen unless
-                       PyFunction_SetDefaults changes. */
-                    Py_DECREF(defs);
-                    Py_DECREF(func);
-                    goto error;
-                }
-                Py_DECREF(defs);
-            }
             if (kwdefaults > 0) {
                 PyObject *defs = PyDict_New();
                 if (defs == NULL) {
@@ -2945,6 +2928,23 @@
                 }
                 Py_DECREF(defs);
             }
+            if (posdefaults > 0) {
+                PyObject *defs = PyTuple_New(posdefaults);
+                if (defs == NULL) {
+                    Py_DECREF(func);
+                    goto error;
+                }
+                while (--posdefaults >= 0)
+                    PyTuple_SET_ITEM(defs, posdefaults, POP());
+                if (PyFunction_SetDefaults(func, defs) != 0) {
+                    /* Can't happen unless
+                       PyFunction_SetDefaults changes. */
+                    Py_DECREF(defs);
+                    Py_DECREF(func);
+                    goto error;
+                }
+                Py_DECREF(defs);
+            }
             PUSH(func);
             DISPATCH();
         }
diff --git a/Python/compile.c b/Python/compile.c
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -1565,6 +1565,8 @@
 
     if (!compiler_decorators(c, decos))
         return 0;
+    if (args->defaults)
+        VISIT_SEQ(c, expr, args->defaults);
     if (args->kwonlyargs) {
         int res = compiler_visit_kwonlydefaults(c, args->kwonlyargs,
                                                 args->kw_defaults);
@@ -1572,8 +1574,6 @@
             return 0;
         kw_default_count = res;
     }
-    if (args->defaults)
-        VISIT_SEQ(c, expr, args->defaults);
     num_annotations = compiler_visit_annotations(c, args, returns);
     if (num_annotations < 0)
         return 0;
diff --git a/Python/importlib.h b/Python/importlib.h
--- a/Python/importlib.h
+++ b/Python/importlib.h
[stripped]

-- 
Repository URL: http://hg.python.org/cpython


More information about the Python-checkins mailing list