From d11b2f323e9dac972f684de0cc51543b13459b85 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 10 Jun 2022 09:26:11 +0100 Subject: [PATCH 1/6] Fix end column offsets for methods calls where value spans several lines. --- Lib/test/test_code.py | 37 ++++++++++++++++++++++++++++++++++++- Python/compile.c | 16 +++++++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 1bb138e7f3243b..a2c2de5d933f76 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -574,6 +574,21 @@ def positions_from_location_table(code): for _ in range(length): yield (line, end_line, col, end_col) +def lines_from_postions(positions): + last = None + res = [] + for l, _, _, _ in positions: + if l != last: + res.append(l) + last = l + return res + +def dedup(lst, prev=object()): + for item in lst: + if item != prev: + yield item + prev = item + def misshappen(): """ @@ -606,6 +621,13 @@ def misshappen(): ) else p +def bug93662(): + example_report_generation_message= ( + """ + """ + ).strip() + raise ValueError() + class CodeLocationTest(unittest.TestCase): @@ -616,10 +638,23 @@ def check_positions(self, func): self.assertEqual(l1, l2) self.assertEqual(len(pos1), len(pos2)) - def test_positions(self): self.check_positions(parse_location_table) self.check_positions(misshappen) + self.check_positions(bug93662) + + def check_lines(self, func): + co = func.__code__ + lines1 = list(dedup(l for (_, _, l) in co.co_lines())) + lines2 = lines_from_postions(positions_from_location_table(co)) + for l1, l2 in zip(lines1, lines2): + self.assertEqual(l1, l2) + self.assertEqual(len(lines1), len(lines2)) + + def test_lines(self): + self.check_lines(parse_location_table) + self.check_lines(misshappen) + self.check_lines(bug93662) if check_impl_detail(cpython=True) and ctypes is not None: diff --git a/Python/compile.c b/Python/compile.c index bbd71936cf3468..accfedfb4b1705 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -4842,8 +4842,12 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) } /* Alright, we can optimize the code. */ VISIT(c, expr, meth->v.Attribute.value); - int old_lineno = c->u->u_lineno; - c->u->u_lineno = meth->end_lineno; + SET_LOC(c, meth); + if (meth->lineno != meth->end_lineno) { + // Make start location match attribute + c->u->u_lineno = meth->end_lineno; + c->u->u_col_offset = meth->end_col_offset - PyUnicode_GetLength(meth->v.Attribute.attr)-1; + } ADDOP_NAME(c, LOAD_METHOD, meth->v.Attribute.attr, names); VISIT_SEQ(c, expr, e->v.Call.args); @@ -4853,8 +4857,13 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) return 0; }; } + SET_LOC(c, e); + if (meth->lineno != meth->end_lineno) { + // Make start location match attribute + c->u->u_lineno = meth->end_lineno; + c->u->u_col_offset = meth->end_col_offset - PyUnicode_GetLength(meth->v.Attribute.attr)-1; + } ADDOP_I(c, CALL, argsl + kwdsl); - c->u->u_lineno = old_lineno; return 1; } @@ -7667,6 +7676,7 @@ write_location_info_short_form(struct assembler* a, int length, int column, int int column_low_bits = column & 7; int column_group = column >> 3; assert(column < 80); + assert(end_column >= column); assert(end_column - column < 16); write_location_first_byte(a, PY_CODE_LOCATION_INFO_SHORT0 + column_group, length); write_location_byte(a, (column_low_bits << 4) | (end_column - column)); From 2412451faf747183b892b16b2f9f3fdb16f997f8 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 10 Jun 2022 10:25:52 +0100 Subject: [PATCH 2/6] Handle erroneous end column offset gracefully. --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index accfedfb4b1705..4f0e97fa5275f2 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7748,7 +7748,7 @@ write_location_info_entry(struct assembler* a, struct instr* i, int isize) } } else if (i->i_end_lineno == i->i_lineno) { - if (line_delta == 0 && column < 80 && end_column - column < 16) { + if (line_delta == 0 && column < 80 && end_column - column < 16 && end_column >= column) { write_location_info_short_form(a, isize, column, end_column); return 1; } From 1c384328ea7c878afab031e001c0816225b2ae72 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 10 Jun 2022 10:31:26 +0100 Subject: [PATCH 3/6] Add news --- .../2022-06-10-10-31-18.gh-issue-93662.-7RSC1.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-06-10-10-31-18.gh-issue-93662.-7RSC1.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-06-10-10-31-18.gh-issue-93662.-7RSC1.rst b/Misc/NEWS.d/next/Core and Builtins/2022-06-10-10-31-18.gh-issue-93662.-7RSC1.rst new file mode 100644 index 00000000000000..e444a00cf7ecc0 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-06-10-10-31-18.gh-issue-93662.-7RSC1.rst @@ -0,0 +1,2 @@ +Make sure that the end column offsets are correct in multi-line method +calls. Previously, the end column could precede the column offset. From 86c7cf3a77d2bdbb2f9dac8c385b928b08c573fa Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 10 Jun 2022 13:18:00 +0100 Subject: [PATCH 4/6] Fix MSVC warnings. --- Python/compile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 4f0e97fa5275f2..c0bdf904243b73 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -4846,7 +4846,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) if (meth->lineno != meth->end_lineno) { // Make start location match attribute c->u->u_lineno = meth->end_lineno; - c->u->u_col_offset = meth->end_col_offset - PyUnicode_GetLength(meth->v.Attribute.attr)-1; + c->u->u_col_offset = meth->end_col_offset - (int)PyUnicode_GetLength(meth->v.Attribute.attr)-1; } ADDOP_NAME(c, LOAD_METHOD, meth->v.Attribute.attr, names); VISIT_SEQ(c, expr, e->v.Call.args); @@ -4861,7 +4861,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) if (meth->lineno != meth->end_lineno) { // Make start location match attribute c->u->u_lineno = meth->end_lineno; - c->u->u_col_offset = meth->end_col_offset - PyUnicode_GetLength(meth->v.Attribute.attr)-1; + c->u->u_col_offset = meth->end_col_offset - (int)PyUnicode_GetLength(meth->v.Attribute.attr)-1; } ADDOP_I(c, CALL, argsl + kwdsl); return 1; From f2429cd59faca2b40d4838413e243440b60699ed Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 13 Jun 2022 17:59:47 +0100 Subject: [PATCH 5/6] Tidy up test --- Lib/test/test_code.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index a2c2de5d933f76..a6857dc4f84120 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -574,21 +574,15 @@ def positions_from_location_table(code): for _ in range(length): yield (line, end_line, col, end_col) -def lines_from_postions(positions): - last = None - res = [] - for l, _, _, _ in positions: - if l != last: - res.append(l) - last = l - return res - def dedup(lst, prev=object()): for item in lst: if item != prev: yield item prev = item +def lines_from_postions(positions): + return dedup(l for (l, _, _, _) in positions) + def misshappen(): """ @@ -646,7 +640,7 @@ def test_positions(self): def check_lines(self, func): co = func.__code__ lines1 = list(dedup(l for (_, _, l) in co.co_lines())) - lines2 = lines_from_postions(positions_from_location_table(co)) + lines2 = list(lines_from_postions(positions_from_location_table(co))) for l1, l2 in zip(lines1, lines2): self.assertEqual(l1, l2) self.assertEqual(len(lines1), len(lines2)) From aac53a8c53ba31d683692eedad24f9ec19d72e72 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 13 Jun 2022 18:04:13 +0100 Subject: [PATCH 6/6] Factor common code into a helper function. --- Python/compile.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index c0bdf904243b73..cdaf7d9c07635b 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -4801,6 +4801,16 @@ is_import_originated(struct compiler *c, expr_ty e) return flags & DEF_IMPORT; } +static void +update_location_to_match_attr(struct compiler *c, expr_ty meth) +{ + if (meth->lineno != meth->end_lineno) { + // Make start location match attribute + c->u->u_lineno = meth->end_lineno; + c->u->u_col_offset = meth->end_col_offset - (int)PyUnicode_GetLength(meth->v.Attribute.attr)-1; + } +} + // Return 1 if the method call was optimized, -1 if not, and 0 on error. static int maybe_optimize_method_call(struct compiler *c, expr_ty e) @@ -4843,11 +4853,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) /* Alright, we can optimize the code. */ VISIT(c, expr, meth->v.Attribute.value); SET_LOC(c, meth); - if (meth->lineno != meth->end_lineno) { - // Make start location match attribute - c->u->u_lineno = meth->end_lineno; - c->u->u_col_offset = meth->end_col_offset - (int)PyUnicode_GetLength(meth->v.Attribute.attr)-1; - } + update_location_to_match_attr(c, meth); ADDOP_NAME(c, LOAD_METHOD, meth->v.Attribute.attr, names); VISIT_SEQ(c, expr, e->v.Call.args); @@ -4858,11 +4864,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e) }; } SET_LOC(c, e); - if (meth->lineno != meth->end_lineno) { - // Make start location match attribute - c->u->u_lineno = meth->end_lineno; - c->u->u_col_offset = meth->end_col_offset - (int)PyUnicode_GetLength(meth->v.Attribute.attr)-1; - } + update_location_to_match_attr(c, meth); ADDOP_I(c, CALL, argsl + kwdsl); return 1; }