From 2715044598c8ddcde42f09b95fda46fb3b3c120c Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 26 Aug 2024 11:20:41 +0100 Subject: [PATCH 1/5] gh-121404: compiler_visit_* --> codegen_visit_* --- Python/compile.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index b1254eff3621c8..96f40e49078c60 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -314,9 +314,9 @@ static int compiler_warn(struct compiler *, location loc, const char *, ...); static int compiler_nameop(struct compiler *, location, identifier, expr_context_ty); static PyCodeObject *compiler_mod(struct compiler *, mod_ty); -static int compiler_visit_stmt(struct compiler *, stmt_ty); -static int compiler_visit_keyword(struct compiler *, keyword_ty); -static int compiler_visit_expr(struct compiler *, expr_ty); +static int codegen_visit_stmt(struct compiler *, stmt_ty); +static int codegen_visit_keyword(struct compiler *, keyword_ty); +static int codegen_visit_expr(struct compiler *, expr_ty); static int codegen_augassign(struct compiler *, stmt_ty); static int codegen_annassign(struct compiler *, stmt_ty); static int codegen_subscript(struct compiler *, expr_ty); @@ -1037,17 +1037,17 @@ codegen_addop_j(instr_sequence *seq, location loc, */ #define VISIT(C, TYPE, V) \ - RETURN_IF_ERROR(compiler_visit_ ## TYPE((C), (V))); + RETURN_IF_ERROR(codegen_visit_ ## TYPE((C), (V))); #define VISIT_IN_SCOPE(C, TYPE, V) \ - RETURN_IF_ERROR_IN_SCOPE((C), compiler_visit_ ## TYPE((C), (V))) + RETURN_IF_ERROR_IN_SCOPE((C), codegen_visit_ ## TYPE((C), (V))) #define VISIT_SEQ(C, TYPE, SEQ) { \ int _i; \ asdl_ ## TYPE ## _seq *seq = (SEQ); /* avoid variable capture */ \ for (_i = 0; _i < asdl_seq_LEN(seq); _i++) { \ TYPE ## _ty elt = (TYPE ## _ty)asdl_seq_GET(seq, _i); \ - RETURN_IF_ERROR(compiler_visit_ ## TYPE((C), elt)); \ + RETURN_IF_ERROR(codegen_visit_ ## TYPE((C), elt)); \ } \ } @@ -1056,7 +1056,7 @@ codegen_addop_j(instr_sequence *seq, location loc, asdl_ ## TYPE ## _seq *seq = (SEQ); /* avoid variable capture */ \ for (_i = 0; _i < asdl_seq_LEN(seq); _i++) { \ TYPE ## _ty elt = (TYPE ## _ty)asdl_seq_GET(seq, _i); \ - if (compiler_visit_ ## TYPE((C), elt) < 0) { \ + if (codegen_visit_ ## TYPE((C), elt) < 0) { \ compiler_exit_scope(C); \ return ERROR; \ } \ @@ -1850,9 +1850,7 @@ codegen_kwonlydefaults(struct compiler *c, location loc, goto error; } ADDOP_LOAD_CONST_NEW(c, loc, mangled); - if (compiler_visit_expr(c, default_) < 0) { - goto error; - } + RETURN_IF_ERROR(codegen_visit_expr(c, default_)); } } if (default_count) { @@ -1869,7 +1867,7 @@ codegen_kwonlydefaults(struct compiler *c, location loc, } static int -compiler_visit_annexpr(struct compiler *c, expr_ty annotation) +codegen_visit_annexpr(struct compiler *c, expr_ty annotation) { location loc = LOC(annotation); ADDOP_LOAD_CONST_NEW(c, loc, _PyAST_ExprAsUnicode(annotation)); @@ -3854,7 +3852,7 @@ codegen_stmt_expr(struct compiler *c, location loc, expr_ty value) } static int -compiler_visit_stmt(struct compiler *c, stmt_ty s) +codegen_visit_stmt(struct compiler *c, stmt_ty s) { switch (s->kind) { @@ -5807,7 +5805,7 @@ codegen_dictcomp(struct compiler *c, expr_ty e) static int -compiler_visit_keyword(struct compiler *c, keyword_ty k) +codegen_visit_keyword(struct compiler *c, keyword_ty k) { VISIT(c, expr, k->value); return SUCCESS; @@ -6035,7 +6033,7 @@ codegen_with(struct compiler *c, stmt_ty s, int pos) } static int -compiler_visit_expr(struct compiler *c, expr_ty e) +codegen_visit_expr(struct compiler *c, expr_ty e) { location loc = LOC(e); switch (e->kind) { @@ -6955,9 +6953,7 @@ codegen_pattern_mapping(struct compiler *c, pattern_ty p, compiler_error(c, LOC(p), e); goto error; } - if (compiler_visit_expr(c, key) < 0) { - goto error; - } + RETURN_IF_ERROR(codegen_visit_expr(c, key)); } // all keys have been checked; there are no duplicates From b732fb70781e2a9fcb81e08977ed6c745fa902a6 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 27 Aug 2024 11:33:08 +0100 Subject: [PATCH 2/5] revert macros, leaks --- Python/compile.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 96f40e49078c60..48d3e7b28cfcb7 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1850,7 +1850,9 @@ codegen_kwonlydefaults(struct compiler *c, location loc, goto error; } ADDOP_LOAD_CONST_NEW(c, loc, mangled); - RETURN_IF_ERROR(codegen_visit_expr(c, default_)); + if (codegen_visit_expr(c, default_) < 0) { + goto error; + } } } if (default_count) { @@ -6953,7 +6955,9 @@ codegen_pattern_mapping(struct compiler *c, pattern_ty p, compiler_error(c, LOC(p), e); goto error; } - RETURN_IF_ERROR(codegen_visit_expr(c, key)); + if (codegen_visit_expr(c, key) < 0) { + goto error; + } } // all keys have been checked; there are no duplicates From 30eb30e3628ca54107df4ab29fad3b064fb36dec Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 27 Aug 2024 11:35:50 +0100 Subject: [PATCH 3/5] remove unused local and unnecessary error label --- Python/compile.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 48d3e7b28cfcb7..cc428fbc4a106d 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1837,22 +1837,18 @@ codegen_kwonlydefaults(struct compiler *c, location loc, Return -1 on error, 0 if no dict pushed, 1 if a dict is pushed. */ - int i; - PyObject *keys = NULL; int default_count = 0; - for (i = 0; i < asdl_seq_LEN(kwonlyargs); i++) { + for (int i = 0; i < asdl_seq_LEN(kwonlyargs); i++) { arg_ty arg = asdl_seq_GET(kwonlyargs, i); expr_ty default_ = asdl_seq_GET(kw_defaults, i); if (default_) { default_count++; PyObject *mangled = compiler_maybe_mangle(c, arg->arg); if (!mangled) { - goto error; + return ERROR; } ADDOP_LOAD_CONST_NEW(c, loc, mangled); - if (codegen_visit_expr(c, default_) < 0) { - goto error; - } + RETURN_IF_ERROR(codegen_visit_expr(c, default_)); } } if (default_count) { @@ -1862,10 +1858,6 @@ codegen_kwonlydefaults(struct compiler *c, location loc, else { return 0; } - -error: - Py_XDECREF(keys); - return ERROR; } static int From fef1cda00898bd9b2e76869806c0f7422498bde7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 27 Aug 2024 11:40:03 +0100 Subject: [PATCH 4/5] use VISIT macro --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index cc428fbc4a106d..7e8ccb8baeaaab 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1848,7 +1848,7 @@ codegen_kwonlydefaults(struct compiler *c, location loc, return ERROR; } ADDOP_LOAD_CONST_NEW(c, loc, mangled); - RETURN_IF_ERROR(codegen_visit_expr(c, default_)); + VISIT(c, expr, default_); } } if (default_count) { From e6e2201009c690cdb24a89e234cc998e3b51aa9a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 27 Aug 2024 12:05:50 +0100 Subject: [PATCH 5/5] split codegen_pattern_mapping_key out of codegen_pattern_mapping to simplify error checking. Used VISIT macro instead of codegen_visit_expr --- Python/compile.c | 74 +++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 7e8ccb8baeaaab..3e762aace96f21 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -6398,7 +6398,7 @@ codegen_annassign(struct compiler *c, stmt_ty s) return SUCCESS; } -/* Raises a SyntaxError and returns 0. +/* Raises a SyntaxError and returns ERROR. If something goes wrong, a different exception may be raised. */ @@ -6869,6 +6869,36 @@ codegen_pattern_class(struct compiler *c, pattern_ty p, pattern_context *pc) return SUCCESS; } +static int +codegen_pattern_mapping_key(struct compiler *c, PyObject *seen, pattern_ty p, Py_ssize_t i) +{ + asdl_expr_seq *keys = p->v.MatchMapping.keys; + asdl_pattern_seq *patterns = p->v.MatchMapping.patterns; + expr_ty key = asdl_seq_GET(keys, i); + if (key == NULL) { + const char *e = "can't use NULL keys in MatchMapping " + "(set 'rest' parameter instead)"; + location loc = LOC((pattern_ty) asdl_seq_GET(patterns, i)); + return compiler_error(c, loc, e); + } + + if (key->kind == Constant_kind) { + int in_seen = PySet_Contains(seen, key->v.Constant.value); + RETURN_IF_ERROR(in_seen); + if (in_seen) { + const char *e = "mapping pattern checks duplicate key (%R)"; + return compiler_error(c, LOC(p), e, key->v.Constant.value); + } + RETURN_IF_ERROR(PySet_Add(seen, key->v.Constant.value)); + } + else if (key->kind != Attribute_kind) { + const char *e = "mapping pattern keys may only match literals and attribute lookups"; + return compiler_error(c, LOC(p), e); + } + VISIT(c, expr, key); + return SUCCESS; +} + static int codegen_pattern_mapping(struct compiler *c, pattern_ty p, pattern_context *pc) @@ -6915,45 +6945,15 @@ codegen_pattern_mapping(struct compiler *c, pattern_ty p, if (seen == NULL) { return ERROR; } - - // NOTE: goto error on failure in the loop below to avoid leaking `seen` for (Py_ssize_t i = 0; i < size; i++) { - expr_ty key = asdl_seq_GET(keys, i); - if (key == NULL) { - const char *e = "can't use NULL keys in MatchMapping " - "(set 'rest' parameter instead)"; - location loc = LOC((pattern_ty) asdl_seq_GET(patterns, i)); - compiler_error(c, loc, e); - goto error; - } - - if (key->kind == Constant_kind) { - int in_seen = PySet_Contains(seen, key->v.Constant.value); - if (in_seen < 0) { - goto error; - } - if (in_seen) { - const char *e = "mapping pattern checks duplicate key (%R)"; - compiler_error(c, LOC(p), e, key->v.Constant.value); - goto error; - } - if (PySet_Add(seen, key->v.Constant.value)) { - goto error; - } - } - - else if (key->kind != Attribute_kind) { - const char *e = "mapping pattern keys may only match literals and attribute lookups"; - compiler_error(c, LOC(p), e); - goto error; - } - if (codegen_visit_expr(c, key) < 0) { - goto error; + if (codegen_pattern_mapping_key(c, seen, p, i) < 0) { + Py_DECREF(seen); + return ERROR; } } + Py_DECREF(seen); // all keys have been checked; there are no duplicates - Py_DECREF(seen); ADDOP_I(c, LOC(p), BUILD_TUPLE, size); ADDOP(c, LOC(p), MATCH_KEYS); @@ -6998,10 +6998,6 @@ codegen_pattern_mapping(struct compiler *c, pattern_ty p, ADDOP(c, LOC(p), POP_TOP); // Subject. } return SUCCESS; - -error: - Py_DECREF(seen); - return ERROR; } static int