
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2018-09-14 17:22 bygvanrossum, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 9338 | merged | ammar2,2018-09-15 23:34 | |
| Messages (10) | |||
|---|---|---|---|
| msg325366 -(view) | Author: Guido van Rossum (gvanrossum)*![]() | Date: 2018-09-14 17:22 | |
Input file with a subtle error: a number where an assignment target is required:for 1 in []: passRun it, it gives a SyntaxError. Note how the caret pointing to the incorrect token is position one to the left of where you'd expect it: File "s.py", line 1 for 1 in []: pass ^SyntaxError: can't assign to literalFor every syntax error I've seen that's produced by ast.c this seems to be the case -- the caret is always positioned 1 too far left.I tried to understand how this is happening but my AST-fu is lacking. It seems this has been happening since ast.c started added column numbers -- in Python 2.7 there's no caret at all, but in 3.4 and later there's a caret and it has the same problem. (Also in 3.3; I don't have 3.2 or older 3.x lying around to test.) | |||
| msg325375 -(view) | Author: Eric V. Smith (eric.smith)*![]() | Date: 2018-09-14 17:51 | |
I'm doing some fairly major surgery on line and column numbers for fixing f-string errors. I'll see if I can include this case, too. | |||
| msg325380 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | Date: 2018-09-14 18:21 | |
I think ast.c is in the right here and there are two complementary bugs in caret printing and the parser.print_error_text does this: while (--offset > 0) PyFile_WriteString(" ", f);which is off-by-one if offset is zero-indexed (which it should be IMO).The parser has an off-by-one error in the other direction. When it reports an error, it reports the offset of the tokenizer's "cur" buffer. This is generally advanced past the location that actually resulted in a parser error. I believe the parser should be fixed to report the error offset as the offset of the start of the token that caused the error. | |||
| msg325382 -(view) | Author: Eric V. Smith (eric.smith)*![]() | Date: 2018-09-14 18:42 | |
I think Benjamin's diagnosis is correct. In which case, my f-string error reporting changes (see#34364) won't intersect with a fix to this issue. | |||
| msg325463 -(view) | Author: Ammar Askar (ammar2)*![]() | Date: 2018-09-15 23:37 | |
Added a PR that is an implementation of Benjamin's suggestion. The column offset returned has been made 0-indexed and errors now point to the start of the parsed token.This makes errors like `def class` show up asdef class ^instead ofdef class ^Also added tests for the original example and some more errors from ast.c | |||
| msg326286 -(view) | Author: Guido van Rossum (gvanrossum)*![]() | Date: 2018-09-24 21:12 | |
New changeset025eb98dc0c1dc27404df6c544fc2944e0fa9f3a by Guido van Rossum (Ammar Askar) in branch 'master':bpo-34683: Make SyntaxError column offsets consistently 1-indexed (gh-9338)https://github.com/python/cpython/commit/025eb98dc0c1dc27404df6c544fc2944e0fa9f3a | |||
| msg326287 -(view) | Author: Guido van Rossum (gvanrossum)*![]() | Date: 2018-09-24 21:13 | |
Fixed bygh-9338. Thanks Ammar! | |||
| msg326288 -(view) | Author: Guido van Rossum (gvanrossum)*![]() | Date: 2018-09-24 21:14 | |
Hm, should this be backported? I think it's safest not to. | |||
| msg326320 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | Date: 2018-09-25 04:53 | |
Wait, why did you make offsets 1-indexed? My suggestion was to make everything zero indexed... | |||
| msg326351 -(view) | Author: Ammar Askar (ammar2)*![]() | Date: 2018-09-25 12:50 | |
There's some context on the github PR but essentially it boiled down to:- It would make it inconsistent with line offsets which are 1-indexed.- Breaking change to a public C API (PyErr_SyntaxLocationObject)- IDLE and other tools that catch syntax errors rely on it being 1-indexed and this would be a breaking change for them. As a quick example of that I found this vim plugin:https://github.com/vim-syntastic/syntastic/commit/6c91e8d802a77005d793d9cdd055c3da29f74a1b | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:05 | admin | set | github: 78864 |
| 2018-09-25 12:50:04 | ammar2 | set | messages: +msg326351 |
| 2018-09-25 04:53:03 | benjamin.peterson | set | messages: +msg326320 |
| 2018-09-24 21:14:40 | gvanrossum | set | messages: +msg326288 |
| 2018-09-24 21:13:59 | gvanrossum | set | status: open -> closed resolution: fixed messages: +msg326287 stage: patch review -> resolved |
| 2018-09-24 21:12:54 | gvanrossum | set | messages: +msg326286 |
| 2018-09-17 18:31:58 | xtreak | set | nosy: +xtreak |
| 2018-09-17 13:11:32 | pablogsal | set | nosy: +pablogsal |
| 2018-09-15 23:37:00 | ammar2 | set | nosy: +ammar2 messages: +msg325463 |
| 2018-09-15 23:34:31 | ammar2 | set | keywords: +patch stage: patch review pull_requests: +pull_request8761 |
| 2018-09-14 18:42:07 | eric.smith | set | messages: +msg325382 |
| 2018-09-14 18:29:34 | gvanrossum | set | nosy: +eric.smith |
| 2018-09-14 18:21:05 | benjamin.peterson | set | nosy: +benjamin.peterson, -eric.smith,xtreak messages: +msg325380 |
| 2018-09-14 17:51:59 | eric.smith | set | nosy: +eric.smith messages: +msg325375 |
| 2018-09-14 17:30:28 | xtreak | set | nosy: +xtreak |
| 2018-09-14 17:24:29 | gvanrossum | set | nosy: +emilyemorehouse |
| 2018-09-14 17:22:22 | gvanrossum | create | |