Skip to content

gh-101400: Fix incorrect lineno in exception message on continue/break which are not in a loop#101413

Merged
iritkatriel merged 7 commits intopython:mainfrom
corona10:gh-101400
Jan 30, 2023
Merged

gh-101400: Fix incorrect lineno in exception message on continue/break which are not in a loop#101413
iritkatriel merged 7 commits intopython:mainfrom
corona10:gh-101400

Conversation

@corona10
Copy link
Member

@corona10 corona10 commented Jan 30, 2023 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot mentioned this pull request Jan 30, 2023
Closed
@corona10 corona10 changed the title gh-101400: Restore continue/break loc for the non-loop case [WIP] gh-101400: Restore continue/break loc for the non-loop case Jan 30, 2023
@corona10 corona10 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Jan 30, 2023
@corona10 corona10 marked this pull request as ready for review January 30, 2023 04:44
@corona10 corona10 changed the title [WIP] gh-101400: Restore continue/break loc for the non-loop case gh-101400: Restore continue/break loc for the non-loop case Jan 30, 2023
@corona10
Copy link
Member Author

corona10 commented Jan 30, 2023

Without patch


======================================================================
FAIL: test_unloop_break_continue (test.test_compile.TestSpecifics.test_unloop_break_continue) (stmt='break')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/user/oss/cpython/Lib/test/test_compile.py", line 856, in test_unloop_break_continue
    self.assertEqual(exc.lineno, 2)
AssertionError: -1 != 2

======================================================================
FAIL: test_unloop_break_continue (test.test_compile.TestSpecifics.test_unloop_break_continue) (stmt='continue')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/user/oss/cpython/Lib/test/test_compile.py", line 856, in test_unloop_break_continue
    self.assertEqual(exc.lineno, 2)
AssertionError: -1 != 2

----------------------------------------------------------------------

@iritkatriel iritkatriel changed the title gh-101400: Restore continue/break loc for the non-loop case gh-101400: Fix wrong lineno in exception message on continue/break which are not in a loop Jan 30, 2023
iritkatriel
iritkatriel reviewed Jan 30, 2023
View reviewed changes
Lib/test/test_compile.py Outdated
source = f"with object() as obj:\n {stmt}"
compile(source, f"<unloop_{stmt}>", "exec")
exc = err_ctx.exception
self.assertEqual(exc.lineno, 2)
Copy link
Member

@iritkatriel iritkatriel Jan 30, 2023

Choose a reason for hiding this comment

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

There are some tests for break and continue outside loops in Lib/test/test_syntax.py, I think the lineno check could be added there. For instance, test_break_outside_loop calls self._check_error() but doesn't pass an expected lineno, though it could. And a similar test for continue could be added next to it.

Copy link
Member Author

@corona10 corona10 Jan 30, 2023

Choose a reason for hiding this comment

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

Done!

@corona10 corona10 requested a review from iritkatriel January 30, 2023 16:04
iritkatriel
iritkatriel reviewed Jan 30, 2023
View reviewed changes
@corona10 corona10 requested a review from iritkatriel January 30, 2023 16:36
iritkatriel
iritkatriel approved these changes Jan 30, 2023
View reviewed changes
Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

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

LGTM!

@iritkatriel iritkatriel changed the title gh-101400: Fix wrong lineno in exception message on continue/break which are not in a loop gh-101400: Fix incorrect lineno in exception message on continue/break which are not in a loop Jan 30, 2023
@iritkatriel iritkatriel merged commit e867c1b into python:main Jan 30, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Jan 30, 2023

Thanks @corona10 for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jan 30, 2023

Sorry, @corona10 and @iritkatriel, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e867c1b753424d8d3f9c9ba0b431d007fd958c80 3.11

@miss-islington
Copy link
Contributor

miss-islington commented Jan 30, 2023

Sorry @corona10 and @iritkatriel, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker e867c1b753424d8d3f9c9ba0b431d007fd958c80 3.10

@corona10
Copy link
Member Author

corona10 commented Jan 30, 2023

@iritkatriel I will create the manual backport.

corona10 added a commit to corona10/cpython that referenced this pull request Jan 31, 2023
@corona10
[3.11] pythongh-101400: Fix incorrect lineno in exception message on …
4e1f5bf 
…continue/break which are not in a loop (pythonGH-101413).

(cherry picked from commit e867c1b)

Co-authored-by: Dong-hee Na <donghee.na@python.org>
@corona10 corona10 mentioned this pull request Jan 31, 2023
Merged
@bedevere-bot
Copy link

bedevere-bot commented Jan 31, 2023

GH-101447 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 31, 2023
@corona10 corona10 mentioned this pull request Jan 31, 2023
Merged
@bedevere-bot
Copy link

bedevere-bot commented Jan 31, 2023

GH-101448 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 31, 2023
@corona10 corona10 deleted the gh-101400 branch January 31, 2023 03:36
corona10 added a commit to corona10/cpython that referenced this pull request Jan 31, 2023
@corona10
[3.10] pythongh-101400: Fix incorrect lineno in exception message on …
17b038c 
…continue/break which are not in a loop (pythonGH-101413).

(cherry picked from commit e867c1b)

Co-authored-by: Dong-hee Na <donghee.na@python.org>
corona10 added a commit to corona10/cpython that referenced this pull request Jan 31, 2023
@corona10
[3.11] pythongh-101400: Fix incorrect lineno in exception message on …
2aa4010 
…continue/break which are not in a loop (pythonGH-101413).

(cherry picked from commit e867c1b)

Co-authored-by: Dong-hee Na <donghee.na@python.org>
mdboom pushed a commit to mdboom/cpython that referenced this pull request Jan 31, 2023
@corona10 @mdboom
pythongh-101400: Fix incorrect lineno in exception message on continu…
bf770d4 
…e/break which are not in a loop (python#101413)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

@iritkatriel iritkatriel

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@corona10 @miss-islington @bedevere-bot @iritkatriel