Skip to content

Conversation

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Apr 26, 2020

@pablogsal
Copy link
Member Author

pablogsal commented Apr 26, 2020

Most of the code is threading down the flags to the parser (commit
784d851) and fix a bug when inlining actions (commit 770fd11). The actual fix is implemented in commit
c970217.
In the future when we don't need to have compatibility with the old parser, we can make the tokenizer return a different token for "<>" and then intercept that in the parser in another alternative.

@gvanrossum
Copy link
Member

Is this needed for alpha 6?

@pablogsal
Copy link
Member Author

Is this needed for alpha 6?

Nop

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks for all the work here for threading through the flags! I think that might solve one of my problems with type comments.

}

int
_PyPegen_check_barry_as_flufl(Parser *p){
Copy link
Member

Choose a reason for hiding this comment

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

PEP 7

Suggested change
_PyPegen_check_barry_as_flufl(Parser *p){
_PyPegen_check_barry_as_flufl(Parser *p) {

Also this function could use a comment explaining what it returns. Apparently nonzero is an error, meaning either <> detected in Guido mode or != detected in Barry mode, so zero means "okay", except it also could mean "exception". (I guess some later check will eventually detect the exception.)

Copy link
Member Author

@pablogsal pablogsal Apr 27, 2020

Choose a reason for hiding this comment

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

(I guess some later check will eventually detect the exception.)

In the exception path, we can return anything IIRC because upstream code will catch that and propagate it. The return value is only used to signal if the subrule for the token should return NULL or the token (indicating that it parses).

Maybe is best to always return an error (0) even after the raise.

Copy link
Member

Choose a reason for hiding this comment

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

Note that 0 is not an error here, it's success. (All this is confused by using strcmp(), which returns 0 for equality.) What happens if you return -1 for errors?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe is best to always return an error (0) even after the raise.

Sorry, this should have been: "return an error (nonzero) even after the raise"

What happens if you return -1 for errors?

The parser reports an error because the sub-rule will return NULL for the token

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Go ahead, maybe we'll make the alpha yet!

@pablogsal pablogsal merged commit 2b74c83 into python:master Apr 27, 2020
@pablogsal pablogsal deleted the bpo-40334-barry branch May 19, 2021 18:59
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.

Failure in test_flufl: '<>' raises SyntaxError even with CO_FUTURE_BARRY_AS_BDFL

4 participants