python.gram: reflect changes in cpython #41

MatthieuDartiailh · 2021-09-28T07:22:09Z

See python/cpython@e5f13ce

I will try to add validation of the line and offsets values in the tests but maybe not before next week.

data/python.gram

tests/python_parser/test_syntax_error_handling.py

pablogsal · 2022-03-30T10:57:12Z

@MatthieuDartiailh You need to rebase this branch on top of main now that #60 has landed.

MatthieuDartiailh · 2022-03-30T10:58:01Z

Will do yes and I need to address the question of the tests too.

python.gram column number fixes

For lower version we only check an error did occur not its message or location

MatthieuDartiailh · 2022-04-04T19:32:20Z

All tests pass locally on 3.10.0, I will try to fix the three broken tests when I get a chance but I would already appreciate a review.

MatthieuDartiailh · 2022-04-04T19:40:32Z

The three failing are related to this rule

| !(NAME STRING | SOFT_KEYWORD) a=disjunction b=expression_without_invalid {
        _PyPegen_check_legacy_stmt(p, a) ? NULL : p->tokens[p->mark-1]->level == 0 ? NULL :
        RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }

but tokenize.py does not trac the level so emulating this will be tricky.

pablogsal · 2022-04-04T20:35:21Z

The three failing are related to this rule
| !(NAME STRING | SOFT_KEYWORD) a=disjunction b=expression_without_invalid {
        _PyPegen_check_legacy_stmt(p, a) ? NULL : p->tokens[p->mark-1]->level == 0 ? NULL :
        RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }
but tokenize.py does not trac the level so emulating this will be tricky.

Yeah, we can ignore it for the time being. We could try to reformulate it using existing information or remove the restriction for now and let it be more noisy.

The underlying issue is not fixed since we do not have access to the right information.

MatthieuDartiailh · 2022-04-05T06:56:29Z

Once this go in I will work on adding the latest improvement to syntax errors:
bpo45716
bpo45764
bpo45727
bpo45450
bpo46836

pablogsal · 2022-04-05T20:23:35Z

Is this now ready for review?

MatthieuDartiailh · 2022-04-06T05:42:20Z

Yes

MatthieuDartiailh · 2022-04-18T14:47:07Z

ping @pablogsal @lysnikolaou

Would it it be possible to get a review for this ?

pablogsal · 2022-04-18T15:17:03Z

ping @pablogsal @lysnikolaou

Would it it be possible to get a review for this ?

Yeah, I will try to get to this this week. As we are close to 3.11b1 I'm getting a ton of extra work these weeks on CPython so I am a bit overwhelmed.

Apologies for the delay :(

pablogsal · 2022-04-18T18:14:20Z

data/python.gram

@@ -171,59 +189,86 @@ class Parser(Parser):
                f"(line {node.lineno})."
            )

+    def get_invalid_target(self, target: Target, node: Optional[ast.AST]) -> Optional[ast.AST]:


Note to self: this mirrors _PyPegen_get_invalid_target

data/python.gram

pablogsal · 2022-04-18T18:28:27Z

data/python.gram

        raise self._build_syntax_error(message, start, end)

    def make_syntax_error(self, message: str) -> None:
        return self._build_syntax_error(message)

-    def raise_syntax_error(self, message: str) -> None:
+    def expect_forced(self, res: Any, expectation: str) -> Optional[tokenize.TokenInfo]:


Where are we using this method? IIRC forced tokens already work

pegen/tests/test_pegen.py

Line 589 in e28fe4f

def test_forced() -> None:

This is needed to get the right error location. CPython report equal start and end for forced token which is not what we do in the default implementation. The default parser only has make_syntax_error which queries the last token and use start and end which is reasonable in general but not in this special case.

Do we have tests covering this difference? Maybe we should add some to test_pegen.py to be explicit about it.

We have 4 tests in test_syntax_error_handling.py failing if I comment this out.

MatthieuDartiailh · 2022-05-11T12:11:11Z

ping @pablogsal

pablogsal

LGTM

Thanks a lot, @MatthieuDartiailh for the patience and for the fantastic work. I know how much this work takes and I wanted to highlight how awesome is that you dedicated a lot of effort to get parity with the latest changes.

I apologized for the time this has been lying around, but the release of 3.11 is proving to be challenging 😅

MatthieuDartiailh · 2022-05-13T18:55:19Z

Thanks @pablogsal !

#64 should be quite easy to review and add next. My other 2 PRs require some more discussions.

pablogsal reviewed Sep 28, 2021

View reviewed changes

data/python.gram Outdated Show resolved Hide resolved

pablogsal reviewed Sep 28, 2021

View reviewed changes

data/python.gram Outdated Show resolved Hide resolved

pablogsal reviewed Sep 28, 2021

View reviewed changes

tests/python_parser/test_syntax_error_handling.py Outdated Show resolved Hide resolved

This was referenced Oct 3, 2021

python.gram: Incorrect column numbers in SyntaxError #47

Open

python.gram column number fixes MatthieuDartiailh/pegen#1

Merged

edemaine mentioned this pull request Oct 19, 2021

python.gram: SyntaxError column numbers to match cpython 3.10 #48

Closed

This was referenced Mar 27, 2022

data/python_parser.py out of sync with data/python.gram #61

Closed

Delayed error inspection #60

Merged

MatthieuDartiailh and others added 7 commits March 31, 2022 21:38

tests: add support for testing line and col offset in error messages

9758af0

tests: fix formatting

4118244

Add some column numbers, rename stop -> end

b47f5b1

Fix lint

5f1fcb1

Remove testing code

efb827a

Compare errors against Python

74a3836

Merge pull request #1 from edemaine/python-columns

a706d1f

python.gram column number fixes

MatthieuDartiailh force-pushed the generator-call-error branch from 0b3e894 to a706d1f Compare April 1, 2022 12:22

MatthieuDartiailh added 10 commits April 4, 2022 21:28

python.gram: improve errors for invalid targets

dae2903

python.gram: improve generic syntax error report

337a91b

python.gram: add location information to indentation errors

846ef26

python.gram: improve and add location to real number checks in match

e2e02a0

python.gram: improve string AST generation

4ac47c6

python.gram: add end postion to generated syntax errors

d21e4e9

python.gram: improve generator as argument syntax error

ff743dd

python.gram: miscellaneous fixes

5f371f1

tests: validate all error messages against Python 3.10

6493637

For lower version we only check an error did occur not its message or location

tests: fix tests

1669b5a

MatthieuDartiailh force-pushed the generator-call-error branch from 8839a2e to 1669b5a Compare April 4, 2022 19:28

tests: add () to missing comma tests so that they pass

7413244

The underlying issue is not fixed since we do not have access to the right information.

MatthieuDartiailh mentioned this pull request Apr 8, 2022

Improved syntax error #64

Merged

MatthieuDartiailh requested review from lysnikolaou and pablogsal April 8, 2022 08:24

pablogsal reviewed Apr 18, 2022

View reviewed changes

data/python.gram Outdated Show resolved Hide resolved

pablogsal reviewed Apr 18, 2022

View reviewed changes

data/python.gram Show resolved Hide resolved

pablogsal reviewed Apr 18, 2022

View reviewed changes

data/python.gram Show resolved Hide resolved

pablogsal reviewed Apr 18, 2022

View reviewed changes

python.gram: address review comments

5794644

pablogsal approved these changes May 13, 2022

View reviewed changes

pablogsal merged commit 995c737 into we-like-parsers:main May 13, 2022

MatthieuDartiailh deleted the generator-call-error branch May 13, 2022 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

python.gram: reflect changes in cpython #41

python.gram: reflect changes in cpython #41

MatthieuDartiailh commented Sep 28, 2021

pablogsal commented Mar 30, 2022

MatthieuDartiailh commented Mar 30, 2022

MatthieuDartiailh commented Apr 4, 2022

MatthieuDartiailh commented Apr 4, 2022

pablogsal commented Apr 4, 2022

MatthieuDartiailh commented Apr 5, 2022

pablogsal commented Apr 5, 2022

MatthieuDartiailh commented Apr 6, 2022

MatthieuDartiailh commented Apr 18, 2022

pablogsal commented Apr 18, 2022

pablogsal Apr 18, 2022

pablogsal Apr 18, 2022 •

edited

Loading

MatthieuDartiailh Apr 18, 2022

pablogsal Apr 18, 2022

MatthieuDartiailh Apr 18, 2022

MatthieuDartiailh commented May 11, 2022

pablogsal left a comment

MatthieuDartiailh commented May 13, 2022

python.gram: reflect changes in cpython #41

python.gram: reflect changes in cpython #41

Conversation

MatthieuDartiailh commented Sep 28, 2021

pablogsal commented Mar 30, 2022

MatthieuDartiailh commented Mar 30, 2022

MatthieuDartiailh commented Apr 4, 2022

MatthieuDartiailh commented Apr 4, 2022

pablogsal commented Apr 4, 2022

MatthieuDartiailh commented Apr 5, 2022

pablogsal commented Apr 5, 2022

MatthieuDartiailh commented Apr 6, 2022

MatthieuDartiailh commented Apr 18, 2022

pablogsal commented Apr 18, 2022

pablogsal Apr 18, 2022

Choose a reason for hiding this comment

pablogsal Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

MatthieuDartiailh Apr 18, 2022

Choose a reason for hiding this comment

pablogsal Apr 18, 2022

Choose a reason for hiding this comment

MatthieuDartiailh Apr 18, 2022

Choose a reason for hiding this comment

MatthieuDartiailh commented May 11, 2022

pablogsal left a comment

Choose a reason for hiding this comment

MatthieuDartiailh commented May 13, 2022

pablogsal Apr 18, 2022 •

edited

Loading