Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

bpo-40334: Do not show error caret if RAISE_SYNTAX_ERROR_NO_COL_OFFSE…#20020

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
pablogsal wants to merge1 commit intopython:masterfrompablogsal:no-col-offset

Conversation

pablogsal
Copy link
Member

@pablogsalpablogsal commentedMay 10, 2020
edited by gvanrossum
Loading

Before:

>>> f(x for x in range(10),)  File"<console>", line1    f(xfor xinrange(10),)^SyntaxError:Generator expression must be parenthesized

With this PR:

>>> f(x for x in range(10),)  File"<console>", line1SyntaxError:Generator expression must be parenthesized

https://bugs.python.org/issue40334

@@ -620,7 +620,7 @@ t_atom[expr_ty]:
incorrect_arguments:
| args ',' '*' { RAISE_SYNTAX_ERROR("iterable argument unpacking follows keyword argument unpacking") }
| expression for_if_clauses ',' [args | expression for_if_clauses] {
RAISE_SYNTAX_ERROR("Generator expression must be parenthesized") }
RAISE_SYNTAX_ERROR_NO_COL_OFFSET("Generator expression must be parenthesized") }
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I made this change in this PR because I was changing this error when I realized that the caret was always pointing to the beginning of the line when usingRAISE_SYNTAX_ERROR_NO_COL_OFFSET

Copy link
Member

Choose a reason for hiding this comment

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

So this is a test of the PR and should not be committed?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

So this is a test of the PR and should not be committed?

I was proposing to commit this in tandem with the other change because currently, the caret points to the end of the call and that seems wrong to me:

>>> f(x for x in range(10), x, y, z)  File "<console>", line 1    f(x for x in range(10), x, y, z)                                   ^SyntaxError: Generator expression must be parenthesized

Copy link
MemberAuthor

@pablogsalpablogsalMay 10, 2020
edited
Loading

Choose a reason for hiding this comment

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

Also, the old parser doesn't show a column offset:

./python -X oldparserPython 3.9.0a6+ (heads/no-collisions:6530eb2be9, May 10 2020, 01:41:04)>>> f(x for x in range(10), x, y, z)  File "<console>", line 1SyntaxError: Generator expression must be parenthesized

Copy link
Member

Choose a reason for hiding this comment

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

But omitting the column seems wrong too (there could be multiple calls on the line). Ideally the caret should point to the first character after the(.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Ideally the caret should point to the first character after the(.

In this case, I agree, but what about something like

f(a, b, c, x for x in range(10), x, y, z)

should it point to the beginning ofx for x in range(10) or to the first caracter after the(?

Copy link
Member

Choose a reason for hiding this comment

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

IMO it's still better if it points to the start than to the end, because that's closer to the function 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 finally solved this mystery, I think.

The reason the old parser doesn't show the caret is because you tried it in the REPL. It seems the old parser doesn't show the source line for errors generated in some later pass (ast.c or the bytecode compiler), because the code that retrieves the source linereads the source file again. Compare these two:

~/cpython$ ./python.exe  -X oldparserPython 3.9.0a6+ (heads/master:f453221c8b, May 12 2020, 10:45:53) [Clang 11.0.0 (clang-1100.0.33.8)] on darwinType "help", "copyright", "credits" or "license" for more information.>>> (pass)  File "<stdin>", line 1    (pass)     ^SyntaxError: invalid syntax>>> f() = 1  File "<stdin>", line 1SyntaxError: cannot assign to function call>>>

Only the first of these shows the source line and the caret, because the error is detected by the (old) parser. The second comes from ast.c and there the attempt to retrieve the source line fails, causing thetext member of the SyntaxError object to be None.

But if you put the second syntax error in a file, the old parser shows the source line and the caret (and as I've shown, the caret is in the right place too).

I'm going to close this PR;@lysnikolaou will fix it properly in#20050.

@pablogsal
Copy link
MemberAuthor

pablogsal commentedMay 10, 2020
edited
Loading

Wow, I am confused about this failing test intest_cmd_line_script:

     def test_syntaxerror_unindented_caret_position(self):         script = "1 + 1 = 2\n"         with support.temp_dir() as script_dir:             script_name = _make_test_script(script_dir, 'script', script)             exitcode, stdout, stderr = assert_python_failure(script_name)             text = io.TextIOWrapper(io.BytesIO(stderr), 'ascii').read()             # Confirm that the caret is located under the first 1 character             self.assertIn("\n    1 + 1 = 2\n    ^", text)

that expects that the caret is on the first character but the error ininvalid_assignment is usingRAISE_SYNTAX_ERROR_NO_COL_OFFSET.@lysnikolaou DoesRAISE_SYNTAX_ERROR_NO_COL_OFFSET means "put the column offset to the beginning" then? If so, I find that somewhat confusing.

@gvanrossum
Copy link
Member

Wow, I am confused about this failing test intest_cmd_line_script:

That's a bug. The old parser puts the caret at the start of the "target". Example:

  File "/Users/guido/cpython/@.py", line 2    if a: 1 + 1 = 2          ^SyntaxError: cannot assign to operator

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaou

@gvanrossumgvanrossumAwaiting requested review from gvanrossum

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@pablogsal@gvanrossum@the-knights-who-say-ni@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp