This is Mark Russell's patch:

[ 1009560 ] Fix @decorator evaluation order

From the description:

Changes in this patch:

- Change Grammar/Grammar to require
newlines between adjacent decorators.

- Fix order of evaluation of decorators
in the C (compile.c) and python
(Lib/compiler/pycodegen.py) compilers

- Add better order of evaluation check
to test_decorators.py (test_eval_order)

- Update the decorator documentation in
the reference manual (improve description
of evaluation order and update syntax
description)

and the comment:

Used Brett's evaluation order (see
http://mail.python.org/pipermail/python-dev/2004-August/047835.html)

(I'm checking this in for Anthony who was having problems getting SF to
talk to him)

Doc/ref/ref7.tex

@@ -318,9 +318,9 @@ section~\ref{types}):
              {[\token{decorators}] "def" \token{funcname} "(" [\token{parameter_list}] ")"
               ":" \token{suite}}
   \production{decorators}
-             {\token{decorator} ([NEWLINE] \token{decorator})* NEWLINE}
+             {\token{decorator}+}
   \production{decorator}
-             {"@" \token{dotted_name} ["(" [\token{argument_list} [","]] ")"]}
+             {"@" \token{dotted_name} ["(" [\token{argument_list} [","]] ")"] NEWLINE}
   \production{parameter_list}
              {(\token{defparameter} ",")*}
   \productioncont{("*" \token{identifier} [, "**" \token{identifier}]}
@@ -352,11 +352,11 @@ Decorator expressions are evaluated when the function is defined, in the scope
 that contains the function definition.  The result must be a callable,
 which is invoked with the function object as the only argument.
 The returned value is bound to the function name instead of the function
-object.  If there are multiple decorators, they are applied in reverse
+object.  Multiple decorators are applied in nested fashion.
-order.  For example, the following code:
+For example, the following code:
 
 \begin{verbatim}
-@f1
+@f1(arg)
 @f2
 def func(): pass
 \end{verbatim}
@@ -365,7 +365,7 @@ is equivalent to:
 
 \begin{verbatim}
 def func(): pass
-func = f2(f1(func))
+func = f1(arg)(f2(func))
 \end{verbatim}
 
 When one or more top-level parameters have the form \var{parameter}

Grammar/Grammar

@@ -28,8 +28,8 @@ single_input: NEWLINE | simple_stmt | compound_stmt NEWLINE
 file_input: (NEWLINE | stmt)* ENDMARKER
 eval_input: testlist NEWLINE* ENDMARKER
 
-decorator: '@' dotted_name [ '(' [arglist] ')' ]
+decorator: '@' dotted_name [ '(' [arglist] ')' ] NEWLINE
-decorators: decorator ([NEWLINE] decorator)* NEWLINE
+decorators: decorator+
 funcdef: [decorators] 'def' NAME parameters ':' suite
 parameters: '(' [varargslist] ')'
 varargslist: (fpdef ['=' test] ',')* ('*' NAME [',' '**' NAME] | '**' NAME) | fpdef ['=' test] (',' fpdef ['=' test])* [',']

Lib/compiler/pycodegen.py

@@ -367,12 +367,12 @@ class CodeGenerator:
 
     def _visitFuncOrLambda(self, node, isLambda=0):
         if not isLambda and node.decorators:
-            for decorator in reversed(node.decorators.nodes):
+            for decorator in node.decorators.nodes:
                 self.visit(decorator)
             ndecorators = len(node.decorators.nodes)
         else:
             ndecorators = 0
-
+ 
         gen = self.FunctionGen(node, self.scopes, isLambda,
                                self.class_name, self.get_module())
         walk(node.code, gen)

Lib/compiler/transformer.py

@@ -201,13 +201,14 @@ class Transformer:
 
     def decorator(self, nodelist):
         # '@' dotted_name [ '(' [arglist] ')' ]
-        assert len(nodelist) in (2, 4, 5)
+        assert len(nodelist) in (3, 5, 6)
         assert nodelist[0][0] == token.AT
+        assert nodelist[-1][0] == token.NEWLINE
 
         assert nodelist[1][0] == symbol.dotted_name
         funcname = self.decorator_name(nodelist[1][1:])
 
-        if len(nodelist) > 2:
+        if len(nodelist) > 3:
             assert nodelist[2][0] == token.LPAR
             expr = self.com_call_function(funcname, nodelist[3])
         else:
@@ -217,16 +218,10 @@ class Transformer:
 
     def decorators(self, nodelist):
         # decorators: decorator ([NEWLINE] decorator)* NEWLINE
-        listlen = len(nodelist)
-        i = 0
         items = []
-        while i < listlen:
+        for dec_nodelist in nodelist:
-            assert nodelist[i][0] == symbol.decorator
+            assert dec_nodelist[0] == symbol.decorator
-            items.append(self.decorator(nodelist[i][1:]))
+            items.append(self.decorator(dec_nodelist[1:]))
-            i += 1
-
-            if i < listlen and nodelist[i][0] == token.NEWLINE:
-                i += 1
         return Decorators(items)
 
     def funcdef(self, nodelist):

Lib/test/test_decorators.py

@@ -40,14 +40,12 @@ def dbcheck(exprstr, globals=None, locals=None):
 def countcalls(counts):
     "Decorator to count calls to a function"
     def decorate(func):
-        name = func.func_name
+        func_name = func.func_name
-        counts[name] = 0
+        counts[func_name] = 0
         def call(*args, **kwds):
-            counts[name] += 1
+            counts[func_name] += 1
             return func(*args, **kwds)
-        # XXX: Would like to say: call.func_name = func.func_name here
+        call.func_name = func_name
-        #      to make nested decorators work in any order, but func_name
-        #      is a readonly attribute
         return call
     return decorate
 
@@ -65,6 +63,7 @@ def memoize(func):
         except TypeError:
             # Unhashable argument
             return func(*args)
+    call.func_name = func.func_name
     return call
 
 # -----------------------------------------------
@@ -126,13 +125,13 @@ class TestDecorators(unittest.TestCase):
         self.assertRaises(DbcheckError, f, 1, None)
 
     def test_memoize(self):
-        # XXX: This doesn't work unless memoize is the last decorator -
-        #      see the comment in countcalls.
         counts = {}
+
         @memoize
         @countcalls(counts)
         def double(x):
             return x * 2
+        self.assertEqual(double.func_name, 'double')
 
         self.assertEqual(counts, dict(double=0))
 
@@ -162,6 +161,11 @@ class TestDecorators(unittest.TestCase):
             codestr = "@%s\ndef f(): pass" % expr
             self.assertRaises(SyntaxError, compile, codestr, "test", "exec")
 
+        # You can't put multiple decorators on a single line:
+        #
+        self.assertRaises(SyntaxError, compile,
+                          "@f1 @f2\ndef f(): pass", "test", "exec")
+
         # Test runtime errors
 
         def unimp(func):
@@ -187,20 +191,74 @@ class TestDecorators(unittest.TestCase):
         self.assertEqual(C.foo.booh, 42)
 
     def test_order(self):
-        # Test that decorators are conceptually applied right-recursively;
+        class C(object):
-        # that means bottom-up
+            @staticmethod
-        def ordercheck(num):
+            @funcattrs(abc=1)
-            def deco(func):
+            def foo(): return 42
-                return lambda: num
+        # This wouldn't work if staticmethod was called first
-            return deco
+        self.assertEqual(C.foo(), 42)
+        self.assertEqual(C().foo(), 42)
 
-        # Should go ordercheck(1)(ordercheck(2)(blah)) which should lead to
+    def test_eval_order(self):
-        # blah() == 1
+        # Evaluating a decorated function involves four steps for each
-        @ordercheck(1)
+        # decorator-maker (the function that returns a decorator):
-        @ordercheck(2)
+        #
-        def blah(): pass
+        #    1: Evaluate the decorator-maker name
-        self.assertEqual(blah(), 1, "decorators are meant to be applied "
+        #    2: Evaluate the decorator-maker arguments (if any)
-                                    "bottom-up")
+        #    3: Call the decorator-maker to make a decorator
+        #    4: Call the decorator
+        #
+        # When there are multiple decorators, these steps should be
+        # performed in the above order for each decorator, but we should
+        # iterate through the decorators in the reverse of the order they
+        # appear in the source.
+
+        actions = []
+
+        def make_decorator(tag):
+            actions.append('makedec' + tag)
+            def decorate(func):
+                actions.append('calldec' + tag)
+                return func
+            return decorate
+
+        class NameLookupTracer (object):
+            def __init__(self, index):
+                self.index = index
+
+            def __getattr__(self, fname):
+                if fname == 'make_decorator':
+                    opname, res = ('evalname', make_decorator)
+                elif fname == 'arg':
+                    opname, res = ('evalargs', str(self.index))
+                else:
+                    assert False, "Unknown attrname %s" % fname
+                actions.append('%s%d' % (opname, self.index))
+                return res
+
+        c1, c2, c3 = map(NameLookupTracer, [ 1, 2, 3 ])
+
+        expected_actions = [ 'evalname1', 'evalargs1', 'makedec1',
+                             'evalname2', 'evalargs2', 'makedec2',
+                             'evalname3', 'evalargs3', 'makedec3',
+                             'calldec3', 'calldec2', 'calldec1' ]
+
+        actions = []
+        @c1.make_decorator(c1.arg)
+        @c2.make_decorator(c2.arg)
+        @c3.make_decorator(c3.arg)
+        def foo(): return 42
+        self.assertEqual(foo(), 42)
+
+        self.assertEqual(actions, expected_actions)
+
+        # Test the equivalence claim in chapter 7 of the reference manual.
+        #
+        actions = []
+        def bar(): return 42
+        bar = c1.make_decorator(c1.arg)(c2.make_decorator(c2.arg)(c3.make_decorator(c3.arg)(bar)))
+        self.assertEqual(bar(), 42)
+        self.assertEqual(actions, expected_actions)
 
 def test_main():
     test_support.run_unittest(TestDecorators)

Modules/parsermodule.c

@@ -2364,7 +2364,7 @@ validate_testlist_gexp(node *tree)
 }
 
 /*  decorator:
- *    '@' dotted_name [ '(' [arglist] ')' ]
+ *    '@' dotted_name [ '(' [arglist] ')' ] NEWLINE
  */
 static int
 validate_decorator(node *tree)
@@ -2372,41 +2372,37 @@ validate_decorator(node *tree)
     int ok;
     int nch = NCH(tree);
     ok = (validate_ntype(tree, decorator) &&
-	  (nch == 2 || nch == 4 || nch == 5) &&
+	  (nch == 3 || nch == 5 || nch == 6) &&
 	  validate_at(CHILD(tree, 0)) &&
-	  validate_dotted_name(CHILD(tree, 1)));
+	  validate_dotted_name(CHILD(tree, 1)) &&
+	  validate_newline(RCHILD(tree, -1)));
 
-    if (ok && nch != 2) {
+    if (ok && nch != 3) {
-	    ok = (validate_lparen(CHILD(tree, 2)) &&
+	ok = (validate_lparen(CHILD(tree, 2)) &&
-		  validate_rparen(RCHILD(tree, -1)));
+	      validate_rparen(RCHILD(tree, -2)));
 
-	    if (ok && nch == 5)
+	if (ok && nch == 6)
-		ok = validate_arglist(CHILD(tree, 3));
+	    ok = validate_arglist(CHILD(tree, 3));
     }
 
     return ok;
 }
-    
+
 /*  decorators:
- *    decorator ([NEWLINE] decorator)* NEWLINE
+ *    decorator+
  */
 static int
 validate_decorators(node *tree)
 {
     int i, nch, ok; 
     nch = NCH(tree);
-    ok = validate_ntype(tree, decorators) && nch >= 2;
+    ok = validate_ntype(tree, decorators) && nch >= 1;
 
-    i = 0;
+    for (i = 0; ok && i < nch; ++i)
-    while (ok && i < nch - 1) {
 	ok = validate_decorator(CHILD(tree, i));
-	if (TYPE(CHILD(tree, i + 1)) == NEWLINE)
-	    ++i;
-	++i;
-    }
 
     return ok;
-}			       
+}
 
 /*  funcdef:
  *      

Python/compile.c

@@ -4107,16 +4107,17 @@ com_decorator_name(struct compiling *c, node *n)
 static void
 com_decorator(struct compiling *c, node *n)
 {
-	/* decorator: '@' dotted_name [ '(' [arglist] ')' ] */
+	/* decorator: '@' dotted_name [ '(' [arglist] ')' ] NEWLINE */
 	int nch = NCH(n);
-	assert(nch >= 2);
+	assert(nch >= 3);
 	REQ(CHILD(n, 0), AT);
+	REQ(RCHILD(n, -1), NEWLINE);
 	com_decorator_name(c, CHILD(n, 1));
 
-	if (nch > 2) {
+	if (nch > 3) {
-		assert(nch == 4 || nch == 5);
+		assert(nch == 5 || nch == 6);
 		REQ(CHILD(n, 2), LPAR);
-		REQ(CHILD(n, nch - 1), RPAR);
+		REQ(RCHILD(n, -2), RPAR);
 		com_call_function(c, CHILD(n, 3));
 	}
 }
@@ -4124,26 +4125,20 @@ com_decorator(struct compiling *c, node *n)
 static int
 com_decorators(struct compiling *c, node *n)
 {
-	int i, nch, ndecorators;
+	int i, nch;
 	
-	/* decorator ([NEWLINE] decorator)* NEWLINE */
+	/* decorator+ */
 	nch = NCH(n);
-	assert(nch >= 2);
+	assert(nch >= 1);
-	REQ(CHILD(n, nch - 1), NEWLINE);
 
-	ndecorators = 0;
+	for (i = 0; i < nch; ++i) {
-	/* the application order for decorators is the reverse of how they are
-	   listed; bottom-up */
-	nch -= 1;
-	for (i = 0; i < nch; i+=1) {
 		node *ch = CHILD(n, i);
-		if (TYPE(ch) != NEWLINE) {
+		REQ(ch, decorator);
-			com_decorator(c, ch);
+
-			++ndecorators;
+		com_decorator(c, ch);
-		}
 	}
 
-	return ndecorators;
+	return nch;
 }
 
 static void
@@ -4151,6 +4146,7 @@ com_funcdef(struct compiling *c, node *n)
 {
 	PyObject *co;
 	int ndefs, ndecorators;
+	
 	REQ(n, funcdef);
 	/*          -6            -5   -4   -3         -2  -1
 	   funcdef: [decorators] 'def' NAME parameters ':' suite */
@@ -4159,7 +4155,7 @@ com_funcdef(struct compiling *c, node *n)
 		ndecorators = com_decorators(c, CHILD(n, 0));
 	else
 		ndecorators = 0;
-	
+
 	ndefs = com_argdefs(c, n);
 	if (ndefs < 0)
 		return;
@@ -4179,11 +4175,13 @@ com_funcdef(struct compiling *c, node *n)
 		else
 			com_addoparg(c, MAKE_FUNCTION, ndefs);
 		com_pop(c, ndefs);
+
 		while (ndecorators > 0) {
 			com_addoparg(c, CALL_FUNCTION, 1);
 			com_pop(c, 1);
-			ndecorators--;
+			--ndecorators;
 		}
+
 		com_addop_varname(c, VAR_STORE, STR(RCHILD(n, -4)));
 		com_pop(c, 1);
 		Py_DECREF(co);

Python/graminit.c

@@ -51,41 +51,40 @@ static arc arcs_3_1[1] = {
 };
 static arc arcs_3_2[2] = {
 	{13, 3},
-	{0, 2},
+	{2, 4},
 };
 static arc arcs_3_3[2] = {
-	{14, 4},
+	{14, 5},
-	{15, 5},
+	{15, 6},
 };
 static arc arcs_3_4[1] = {
-	{15, 5},
+	{0, 4},
 };
 static arc arcs_3_5[1] = {
-	{0, 5},
+	{15, 6},
 };
-static state states_3[6] = {
+static arc arcs_3_6[1] = {
+	{2, 4},
+};
+static state states_3[7] = {
 	{1, arcs_3_0},
 	{1, arcs_3_1},
 	{2, arcs_3_2},
 	{2, arcs_3_3},
 	{1, arcs_3_4},
 	{1, arcs_3_5},
+	{1, arcs_3_6},
 };
 static arc arcs_4_0[1] = {
 	{10, 1},
 };
 static arc arcs_4_1[2] = {
-	{2, 2},
 	{10, 1},
+	{0, 1},
 };
-static arc arcs_4_2[2] = {
+static state states_4[2] = {
-	{10, 1},
-	{0, 2},
-};
-static state states_4[3] = {
 	{1, arcs_4_0},
 	{2, arcs_4_1},
-	{2, arcs_4_2},
 };
 static arc arcs_5_0[2] = {
 	{16, 1},
@@ -1618,9 +1617,9 @@ static dfa dfas[74] = {
 	 "\204\050\014\000\000\000\200\012\176\231\040\007\040\000\000\206\220\064\041\000"},
 	{258, "eval_input", 0, 3, states_2,
 	 "\000\040\010\000\000\000\000\000\000\000\000\000\040\000\000\206\220\064\001\000"},
-	{259, "decorator", 0, 6, states_3,
+	{259, "decorator", 0, 7, states_3,
 	 "\000\010\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"},
-	{260, "decorators", 0, 3, states_4,
+	{260, "decorators", 0, 2, states_4,
 	 "\000\010\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"},
 	{261, "funcdef", 0, 7, states_5,
 	 "\000\010\004\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"},