Skip to content

py: Implement decorator syntax relaxation from CPython 3.9. #8503

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.

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 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 19 additions & 36 deletions py/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -857,17 +857,8 @@ STATIC qstr compile_classdef_helper(compiler_t *comp, mp_parse_node_struct_t *pn
}

// returns true if it was a built-in decorator (even if the built-in had an error)
STATIC bool compile_built_in_decorator(compiler_t *comp, size_t name_len, mp_parse_node_t *name_nodes, uint *emit_options) {
if (MP_PARSE_NODE_LEAF_ARG(name_nodes[0]) != MP_QSTR_micropython) {
return false;
}

if (name_len != 2) {
compile_syntax_error(comp, name_nodes[0], MP_ERROR_TEXT("invalid micropython decorator"));
return true;
}

qstr attr = MP_PARSE_NODE_LEAF_ARG(name_nodes[1]);
STATIC bool compile_built_in_decorator(compiler_t *comp, mp_parse_node_t name_node, uint *emit_options) {
qstr attr = MP_PARSE_NODE_LEAF_ARG(name_node);
if (attr == MP_QSTR_bytecode) {
*emit_options = MP_EMIT_OPT_BYTECODE;
#if MICROPY_EMIT_NATIVE
Expand All @@ -888,17 +879,17 @@ STATIC bool compile_built_in_decorator(compiler_t *comp, size_t name_len, mp_par
#endif
#endif
} else {
compile_syntax_error(comp, name_nodes[1], MP_ERROR_TEXT("invalid micropython decorator"));
compile_syntax_error(comp, name_node, MP_ERROR_TEXT("invalid micropython decorator"));
}

#if MICROPY_EMIT_NATIVE && MICROPY_DYNAMIC_COMPILER
if (*emit_options == MP_EMIT_OPT_NATIVE_PYTHON || *emit_options == MP_EMIT_OPT_VIPER) {
if (emit_native_table[mp_dynamic_compiler.native_arch] == NULL) {
compile_syntax_error(comp, name_nodes[1], MP_ERROR_TEXT("invalid arch"));
compile_syntax_error(comp, name_node, MP_ERROR_TEXT("invalid arch"));
}
} else if (*emit_options == MP_EMIT_OPT_ASM) {
if (emit_asm_table[mp_dynamic_compiler.native_arch] == NULL) {
compile_syntax_error(comp, name_nodes[1], MP_ERROR_TEXT("invalid arch"));
compile_syntax_error(comp, name_node, MP_ERROR_TEXT("invalid arch"));
}
}
#endif
Expand All @@ -920,30 +911,22 @@ STATIC void compile_decorated(compiler_t *comp, mp_parse_node_struct_t *pns) {
assert(MP_PARSE_NODE_IS_STRUCT_KIND(nodes[i], PN_decorator)); // should be
mp_parse_node_struct_t *pns_decorator = (mp_parse_node_struct_t *)nodes[i];

// nodes[0] contains the decorator function, which is a dotted name
mp_parse_node_t *name_nodes;
size_t name_len = mp_parse_node_extract_list(&pns_decorator->nodes[0], PN_dotted_name, &name_nodes);

// check for built-in decorators
if (compile_built_in_decorator(comp, name_len, name_nodes, &emit_options)) {
// this was a built-in
num_built_in_decorators += 1;
assert(MP_PARSE_NODE_STRUCT_NUM_NODES(pns_decorator) == 1);
mp_parse_node_t decorator_arg = pns_decorator->nodes[0];

} else {
// not a built-in, compile normally

// compile the decorator function
compile_node(comp, name_nodes[0]);
for (size_t j = 1; j < name_len; j++) {
assert(MP_PARSE_NODE_IS_ID(name_nodes[j])); // should be
EMIT_ARG(attr, MP_PARSE_NODE_LEAF_ARG(name_nodes[j]), MP_EMIT_ATTR_LOAD);
}

// nodes[1] contains arguments to the decorator function, if any
if (!MP_PARSE_NODE_IS_NULL(pns_decorator->nodes[1])) {
// call the decorator function with the arguments in nodes[1]
compile_node(comp, pns_decorator->nodes[1]);
if (MP_PARSE_NODE_IS_STRUCT_KIND(decorator_arg, PN_built_in_decorator)) {
mp_parse_node_struct_t *decorator_arg_s = (mp_parse_node_struct_t *)decorator_arg;
assert(MP_PARSE_NODE_STRUCT_NUM_NODES(decorator_arg_s) == 2);
// The first child node must be the ID "micropython" (enforced by
// the parser). We just need to have a look at the second child.
mp_parse_node_t name_node = decorator_arg_s->nodes[1];
if (compile_built_in_decorator(comp, name_node, &emit_options)) {
num_built_in_decorators += 1;
}
} else {
// decorator_arg is something derived from a namedexpr_test rule.
// Compile it normally.
compile_node(comp, decorator_arg);
}
}

Expand Down
8 changes: 6 additions & 2 deletions py/grammar.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ DEF_RULE_NC(file_input_3, or(2), tok(NEWLINE), rule(stmt))
DEF_RULE_NC(eval_input, and_ident(2), rule(testlist), opt_rule(eval_input_2))
DEF_RULE_NC(eval_input_2, and(1), tok(NEWLINE))

// decorator: '@' dotted_name [ '(' [arglist] ')' ] NEWLINE
// decorator: '@' decorator_arg NEWLINE
// decorator_arg: built_in_decorator | namedexpr_test
// built_in_decorator: 'micropython' '.' NAME
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be cleaner/simpler to define the grammar like this:

decorator: '@' namedexpr_test NEWLINE

and then handle detection and compilation of the builtin decorator in py/compile.c (that way it can potentially support more complex builtin decorators, eg @micropython.bytecode(opt=2)).

// decorators: decorator+
// decorated: decorators (classdef | funcdef | async_funcdef)
// funcdef: 'def' NAME parameters ['->' test] ':' suite
Expand All @@ -62,7 +64,9 @@ DEF_RULE_NC(eval_input_2, and(1), tok(NEWLINE))
// varargslist: vfpdef ['=' test] (',' vfpdef ['=' test])* [',' ['*' [vfpdef] (',' vfpdef ['=' test])* [',' '**' vfpdef] | '**' vfpdef]] | '*' [vfpdef] (',' vfpdef ['=' test])* [',' '**' vfpdef] | '**' vfpdef
// vfpdef: NAME

DEF_RULE_NC(decorator, and(4), tok(OP_AT), rule(dotted_name), opt_rule(trailer_paren), tok(NEWLINE))
DEF_RULE_NC(decorator, and(3), tok(OP_AT), rule(decorator_arg), tok(NEWLINE))
DEF_RULE_NC(decorator_arg, or(2), rule(built_in_decorator), rule(namedexpr_test))
DEF_RULE_NC(built_in_decorator, and(3), tok(NAME), tok(DEL_PERIOD), tok(NAME))
DEF_RULE_NC(decorators, one_or_more, rule(decorator))
DEF_RULE(decorated, c(decorated), and_ident(2), rule(decorators), rule(decorated_body))
#if MICROPY_PY_ASYNC_AWAIT
Expand Down
5 changes: 5 additions & 0 deletions py/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,11 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) {
mp_token_kind_t tok_kind = rule_arg[i] & RULE_ARG_ARG_MASK;
if (lex->tok_kind == tok_kind) {
// matched token
if (rule_id == RULE_built_in_decorator && i == 0 && qstr_from_strn(lex->vstr.buf, lex->vstr.len) != MP_QSTR_micropython) {
// The first NAME of a built-in decorator must be "micropython"
backtrack = true;
goto next_rule;
}
if (tok_kind == MP_TOKEN_NAME) {
push_result_token(&parser, rule_id);
}
Expand Down
68 changes: 67 additions & 1 deletion tests/basics/decorator.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,71 @@
# test decorators
### Test decorator syntax.

###############################################################################
### Test for syntax errors.
###############################################################################
def test_for_syntax_error(code):
try:
exec(code)
except SyntaxError:
print("SyntaxError")

decs = [
"x,",
"x, y",
"x = y",
"pass",
"import sys",
"@dec"
]

for dec in decs:
code = "@{}\ndef foo(): pass".format(dec)
test_for_syntax_error(code)

###############################################################################
### Test for type errors (previously syntax errors before Python 3.9).
###############################################################################
def test_for_type_error(code):
try:
exec(code)
except TypeError:
print("TypeError")

decs = [
"[1, 2][-1]",
"(1, 2)",
"True",
"...",
"None"
]

for dec in decs:
code = "@{}\ndef foo(): pass".format(dec)
test_for_type_error(code)

###############################################################################
### Test for name errors. Tests added to make sure the compiler does not
### confuse anything as a built-in decorator.
###############################################################################
def test_for_name_error(code):
try:
exec(code)
except NameError:
print("NameError")

decs = [
"some",
"some.dotted",
"some.dotted.name"
]

for dec in decs:
code = "@{}\ndef foo(): pass".format(dec)
test_for_name_error(code)

###############################################################################
### Test valid decorators.
###############################################################################
def dec(f):
print('dec')
return f
Expand Down
17 changes: 17 additions & 0 deletions tests/basics/decorator.py.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
SyntaxError
SyntaxError
SyntaxError
SyntaxError
SyntaxError
SyntaxError
TypeError
TypeError
TypeError
TypeError
TypeError
NameError
NameError
NameError
dec
dec_arg
dec
4 changes: 2 additions & 2 deletions tests/cmdline/cmd_parsetree.py.exp
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@
id(h)
[ 13] \(rule\|atom_expr_normal\)(44) (n=2)
[ 13] literal const(\.\+)
[ 13] \(rule\|atom_expr_trailers\)(142) (n=2)
[ 13] \(rule\|atom_expr_trailers\)(144) (n=2)
[ 13] \(rule\|trailer_period\)(50) (n=1)
id(format)
[ 13] \(rule\|trailer_paren\)(48) (n=1)
[ 13] \(rule\|arglist\)(164) (n=1)
[ 13] \(rule\|arglist\)(166) (n=1)
id(b)
----------------
File cmdline/cmd_parsetree.py, code block '<module>' (descriptor: \.\+, bytecode @\.\+ 62 bytes)
Expand Down