Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-44317: Improve tokenizer errors with more informative locations #26555

Merged
merged 5 commits into from Jul 10, 2021

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 5, 2021

Parser/tokenizer.c Outdated Show resolved Hide resolved
Parser/tokenizer.c Outdated Show resolved Hide resolved
Parser/tokenizer.c Outdated Show resolved Hide resolved
Parser/tokenizer.c Outdated Show resolved Hide resolved
@@ -1089,7 +1083,14 @@ syntaxerror(struct tok_state *tok, const char *format, ...)
if (!errtext) {
goto error;
}
int offset = (int)PyUnicode_GET_LENGTH(errtext);

if (col_offset == 0) {
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 7, 2021

Choose a reason for hiding this comment

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

What if error occurred at the beginning of the line?

Copy link
Member Author

@pablogsal pablogsal Jun 7, 2021

Choose a reason for hiding this comment

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

What if error occurred at the beginning of the line?

Oh, I was thinking here about line numbers. This should have been -1. Thanks for the catch

@@ -1552,6 +1585,7 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end)
/* Number */
if (isdigit(c)) {
if (c == '0') {
const char* number_start = tok->cur;
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 7, 2021

Choose a reason for hiding this comment

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

Cat tok->start be used instead of a new variable?

@@ -1606,6 +1640,8 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end)
if (c != '0' && c != '1') {
tok_backup(tok, c);
if (isdigit(c)) {
// Move to the actual current token that is incorrect
tok_nextc(tok);
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 7, 2021

Choose a reason for hiding this comment

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

Same as above, tok_nextc() cancels tok_backup().

@@ -1639,6 +1675,7 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end)
}
c = tok_nextc(tok);
}
char* zeros_end = tok->cur;
Copy link
Member

@serhiy-storchaka serhiy-storchaka Jun 7, 2021

Choose a reason for hiding this comment

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

Cat tok->start be used instead of a new variable?

Copy link
Member Author

@pablogsal pablogsal Jun 7, 2021

Choose a reason for hiding this comment

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

Not here because we want to highlight only the zeros and tok->cur points at the end of the number

Parser/tokenizer.c Outdated Show resolved Hide resolved
pablogsal and others added 2 commits Jun 7, 2021
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Parser/tokenizer.c Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jul 9, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jul 9, 2021
@pablogsal pablogsal closed this Jul 10, 2021
@pablogsal pablogsal reopened this Jul 10, 2021
@pablogsal pablogsal merged commit f24777c into python:main Jul 10, 2021
21 checks passed
@pablogsal pablogsal deleted the bpo-44317 branch Jul 10, 2021
@miss-islington
Copy link
Contributor

miss-islington commented Jul 10, 2021

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2021
…ythonGH-26555)

(cherry picked from commit f24777c)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Jul 10, 2021

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

pablogsal added a commit that referenced this pull request Jul 10, 2021
…H-26555) (GH-27079)

(cherry picked from commit f24777c)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants