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

Validate Missing performSelfAnalysis call. #987

Closed
DonaldWong opened this issue Jul 11, 2019 · 5 comments · Fixed by #988
Closed

Validate Missing performSelfAnalysis call. #987

DonaldWong opened this issue Jul 11, 2019 · 5 comments · Fixed by #988
Labels

Comments

@DonaldWong
Copy link

DonaldWong commented Jul 11, 2019

Though in tutorial where clause in select works, a similar option with subrule fails like this:

    $.RULE("struct", () => {
        $.CONSUME(Identifier)
        $.CONSUME(LCurly)
        $.OPTION(() => {
            $.SUBRULE($.member)
        })
        $.CONSUME(RCurly)
    })

    $.RULE("member", () => {
        $.CONSUME(Semicolon)
    })

The option is not working for "abc{}". It always expects semicolon.

For full test file, please refer to here

Any idea?

@DonaldWong DonaldWong changed the title Parsing fails in simple option with subrule Parse fails in simple option with subrule Jul 12, 2019
@bd82
Copy link
Member

bd82 commented Jul 12, 2019

You forgot to call this.performSelfAnalysis();

@DonaldWong
Copy link
Author

Oh. Thanks for taking time answer my silly question.

@bd82
Copy link
Member

bd82 commented Jul 12, 2019

I never figured out how to assert that performSelfAnalysis was invoked without cause a performance regression. But now that I think about it perhaps this inspection can be added to the input setter.

@bd82 bd82 changed the title Parse fails in simple option with subrule Validate Missing performSelfAnalysis call. Jul 12, 2019
@bd82 bd82 closed this as completed in #988 Jul 12, 2019
@bd82
Copy link
Member

bd82 commented Jul 12, 2019

Any future similar issues would now be automatically detected by Chevrotain. 😄
See: #988

I am afraid this could be a breaking change for a small subset of existing Chevrotain Parsers so I will need to release a major version.

@bd82
Copy link
Member

bd82 commented Jul 12, 2019

Re-opening until this is released and documented as a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants