From 0ccff074cd806bc7e963a1e0ff6ce25e0d11cce9 Mon Sep 17 00:00:00 2001 From: "Michael W. Hudson" Date: Tue, 17 Aug 2004 17:29:16 +0000 Subject: [PATCH] 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 | 12 ++--- Grammar/Grammar | 4 +- Lib/compiler/pycodegen.py | 4 +- Lib/compiler/transformer.py | 17 +++--- Lib/test/test_decorators.py | 100 ++++++++++++++++++++++++++++-------- Modules/parsermodule.c | 32 +++++------- Python/compile.c | 40 +++++++-------- Python/graminit.c | 29 +++++------ 8 files changed, 142 insertions(+), 96 deletions(-) diff --git a/Doc/ref/ref7.tex b/Doc/ref/ref7.tex index 1d7b8604b6c..88c548c1c52 100644 --- a/Doc/ref/ref7.tex +++ b/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 -order. For example, the following code: +object. Multiple decorators are applied in nested fashion. +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} diff --git a/Grammar/Grammar b/Grammar/Grammar index 111ebe04ad5..8a7eb59243e 100644 --- a/Grammar/Grammar +++ b/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] ')' ] -decorators: decorator ([NEWLINE] decorator)* NEWLINE +decorator: '@' dotted_name [ '(' [arglist] ')' ] NEWLINE +decorators: decorator+ funcdef: [decorators] 'def' NAME parameters ':' suite parameters: '(' [varargslist] ')' varargslist: (fpdef ['=' test] ',')* ('*' NAME [',' '**' NAME] | '**' NAME) | fpdef ['=' test] (',' fpdef ['=' test])* [','] diff --git a/Lib/compiler/pycodegen.py b/Lib/compiler/pycodegen.py index e5d1ff175a9..02e77640474 100644 --- a/Lib/compiler/pycodegen.py +++ b/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) diff --git a/Lib/compiler/transformer.py b/Lib/compiler/transformer.py index 53a0fda7c47..cdeb5ffdc22 100644 --- a/Lib/compiler/transformer.py +++ b/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: - assert nodelist[i][0] == symbol.decorator - items.append(self.decorator(nodelist[i][1:])) - i += 1 - - if i < listlen and nodelist[i][0] == token.NEWLINE: - i += 1 + for dec_nodelist in nodelist: + assert dec_nodelist[0] == symbol.decorator + items.append(self.decorator(dec_nodelist[1:])) return Decorators(items) def funcdef(self, nodelist): diff --git a/Lib/test/test_decorators.py b/Lib/test/test_decorators.py index 8010ea37f3d..c4eb8beb025 100644 --- a/Lib/test/test_decorators.py +++ b/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 - counts[name] = 0 + func_name = func.func_name + 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 - # to make nested decorators work in any order, but func_name - # is a readonly attribute + call.func_name = func_name 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; - # that means bottom-up - def ordercheck(num): - def deco(func): - return lambda: num - return deco + class C(object): + @staticmethod + @funcattrs(abc=1) + def foo(): return 42 + # This wouldn't work if staticmethod was called first + self.assertEqual(C.foo(), 42) + self.assertEqual(C().foo(), 42) - # Should go ordercheck(1)(ordercheck(2)(blah)) which should lead to - # blah() == 1 - @ordercheck(1) - @ordercheck(2) - def blah(): pass - self.assertEqual(blah(), 1, "decorators are meant to be applied " - "bottom-up") + def test_eval_order(self): + # Evaluating a decorated function involves four steps for each + # decorator-maker (the function that returns a decorator): + # + # 1: Evaluate the decorator-maker name + # 2: Evaluate the decorator-maker arguments (if any) + # 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) diff --git a/Modules/parsermodule.c b/Modules/parsermodule.c index 81d96e16a79..5f53982627d 100644 --- a/Modules/parsermodule.c +++ b/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) { - ok = (validate_lparen(CHILD(tree, 2)) && - validate_rparen(RCHILD(tree, -1))); + if (ok && nch != 3) { + ok = (validate_lparen(CHILD(tree, 2)) && + validate_rparen(RCHILD(tree, -2))); - if (ok && nch == 5) - ok = validate_arglist(CHILD(tree, 3)); + if (ok && nch == 6) + 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; - while (ok && i < nch - 1) { + for (i = 0; ok && i < nch; ++i) ok = validate_decorator(CHILD(tree, i)); - if (TYPE(CHILD(tree, i + 1)) == NEWLINE) - ++i; - ++i; - } return ok; -} +} /* funcdef: * diff --git a/Python/compile.c b/Python/compile.c index 79620c209fe..fbb91f792c8 100644 --- a/Python/compile.c +++ b/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) { - assert(nch == 4 || nch == 5); + if (nch > 3) { + 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); - REQ(CHILD(n, nch - 1), NEWLINE); + assert(nch >= 1); - ndecorators = 0; - /* the application order for decorators is the reverse of how they are - listed; bottom-up */ - nch -= 1; - for (i = 0; i < nch; i+=1) { + for (i = 0; i < nch; ++i) { node *ch = CHILD(n, i); - if (TYPE(ch) != NEWLINE) { - com_decorator(c, ch); - ++ndecorators; - } + REQ(ch, decorator); + + 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); diff --git a/Python/graminit.c b/Python/graminit.c index 171809658cf..f4f207e1b75 100644 --- a/Python/graminit.c +++ b/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}, - {15, 5}, + {14, 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] = { - {10, 1}, - {0, 2}, -}; -static state states_4[3] = { +static state states_4[2] = { {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"},