Skip to content

Conversation

@isidentical
Copy link
Member

@isidentical isidentical commented Apr 29, 2020

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 bpo-40334: Improve column offsets for thrown syntax errors by Pegen Apr 29, 2020
@isidentical isidentical requested a review from lysnikolaou April 29, 2020 05:22
Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

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

@gvanrossum
Copy link
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
Copy link
Member Author

@gvanrossum resolved

@pablogsal
Copy link
Member

pablogsal commented May 1, 2020

+77 −109

Best way of fixing a bug! 🎉

@pablogsal
Copy link
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
Copy link
Member

pablogsal commented May 1, 2020

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
Copy link
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants