◐ Shell
reader mode source ↗
Skip to content

bpo-40334: Improve column offsets for thrown syntax errors by Pegen#19782

Merged
pablogsal merged 4 commits into
python:masterfrom
isidentical:bpo-40344
May 1, 2020
Merged

bpo-40334: Improve column offsets for thrown syntax errors by Pegen#19782
pablogsal merged 4 commits into
python:masterfrom
isidentical:bpo-40344

Conversation

@isidentical

@isidentical isidentical commented Apr 29, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

I've been trying to figure out a good way to handle ast.c errors, but no luck so far. I've splitted them to their own test and fixed remaining offset errors to ensure full compatibility with the old parser.

https://bugs.python.org/issue40334

@isidentical isidentical requested a review from pablogsal as a code owner April 29, 2020 01:29
@isidentical isidentical changed the title bpo-40344: Improve column offsets for thrown syntax errors by Pegen Apr 29, 2020
@isidentical isidentical requested a review from lysnikolaou April 29, 2020 05:22
lysnikolaou
lysnikolaou reviewed Apr 29, 2020
View reviewed changes

@lysnikolaou lysnikolaou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Thanks @isidentical for spending time on this! It's a really nice improvement.

@gvanrossum

gvanrossum commented May 1, 2020

Copy link
Copy Markdown
Member

@isidentical there's a merge conflict -- you need to merge master and run make regen-pegen. Then I'm good with merging this.

@isidentical

isidentical commented May 1, 2020

Copy link
Copy Markdown
Member Author

@gvanrossum resolved

@pablogsal

pablogsal commented May 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

+77 −109

Best way of fixing a bug! 🎉

@pablogsal

pablogsal commented May 1, 2020

Copy link
Copy Markdown
Member

The travis target is stucked, closing and reopening to re-trigger the CI

@pablogsal pablogsal closed this May 1, 2020
@pablogsal pablogsal reopened this May 1, 2020
@pablogsal

pablogsal commented May 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Hummm, there is a bunch of warnings now after the last rebasing:

/home/travis/build/python/cpython/Parser/pegen/pegen.h:194:59: warning: incompatible pointer to integer conversion passing 'char [45]' to parameter of type 'int' [-Wint-conversion]
        return _PyPegen_raise_error(p, PyExc_SyntaxError, "%s only supported in Python 3.%i and greater",
                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@isidentical could you take a look? Seems that we are passing the error message incorrectly with this PR

@isidentical

isidentical commented May 1, 2020

Copy link
Copy Markdown
Member Author

Seems that we are passing the error message incorrectly with this PR

Yup, this PR changes signature of _PyPegen_raise_error. I missed that usage when I rebased. Now, it should be fixed.

@pablogsal pablogsal merged commit 76c1b4d into python:master May 1, 2020
This was referenced May 1, 2020
Closed
Closed
@lysnikolaou lysnikolaou mentioned this pull request May 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@lysnikolaou lysnikolaou

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@isidentical @gvanrossum @pablogsal @lysnikolaou @the-knights-who-say-ni @bedevere-bot