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

Breaking change in 2.7.0: false positive 'expression inside repetition is non-progressing and will repeat infinitely' #881

Open
reivilibre opened this issue Jul 5, 2023 · 3 comments

Comments

@reivilibre
Copy link

Describe the bug
Pest 2.7.0 seems to have introduced a breaking change that causes my library to fail to compile. (Previously I was using 2.5.5, I have confirmed 2.5.7 to work and cannot comment on 2.6.* because that series seems to have been yanked from the crate registry).

To Reproduce
Steps to reproduce the behavior:

The following snippet illustrates the issue and can be tried on the pest homepage (I have cut out some unrelated productions):

nlOrEoi = _{ NEWLINE | &EOI }

ws = _{ " " | "\t" }
ws_nocnl = _{ " " | "\t" }

// Line ending, with allowed blank lines.
lineEnd = _{ ws_nocnl* ~ nlOrEoi ~ (!EOI ~ ws* ~ nlOrEoi)* }

This gives an error (taken from my real project, but the snippet is representative):

error: proc-macro derive panicked
  --> project/src/parser.rs:20:10
   |
20 | #[derive(Parser)]
   |          ^^^^^^
   |
   = help: message: grammar error
           
             --> 97:36
              |
           97 | lineEnd = _{ ws_nocnl* ~ nlOrEoi ~ (!EOI ~ ws* ~ nlOrEoi)* }
              |                                    ^---------------------^
              |
              = expression inside repetition is non-progressing and will repeat infinitely

Expected behavior

This expression is actually always progressing, though I can see why the compiler is unable to prove it — were it not for !EOI then I suspect it would be right.
The compiler should not raise an error on this, particularly since it was working in a previous (semver-compatible) version of Pest.

Additional context

The grammar may look a bit unnecessarily convoluted and academic, but the real version deals with e.g. comments etc and has a bit more complexity to it. (Definitely not saying there isn't a simpler way of doing this, but I can't easily see one :))

@reivilibre reivilibre added the bug label Jul 5, 2023
@reivilibre
Copy link
Author

Well, no need to fix it just for me, I found a clearer way to do what I wanted that doesn't cause this error:

lineEnd = _{ ws_nocnl* ~ nlOrEoi ~ (ws* ~ NEWLINE)* ~ (ws* ~ &EOI)? }

However maybe this is biting other people, so it's up to you :).

@tomtau
Copy link
Contributor

tomtau commented Jul 6, 2023

@reivilibre I'm guessing it may be due to this PR: https://github.com/pest-parser/pest/pull/848/files that landed in 2.6.X I think
@Tartasprint was trying to capture more cases: #851

@reivilibre do you think this is a false positive or should we convert this into Q&A just in case people run into that issue?

@Tartasprint
Copy link
Contributor

I do not think this is easy to fix.
It is easy to understand why the expression makes progress, but it requires an in-depth analysis for a program.

To make this possible I suggest (as in #851 ) making annotations to the AST of the Pest file being read at the verification stage.

I do want to make progress on this, but I won't be able to make any until mid-November.

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

No branches or pull requests

3 participants