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

Control flow in {? } blocks #246

Closed
piegamesde opened this issue Jan 13, 2021 · 3 comments
Closed

Control flow in {? } blocks #246

piegamesde opened this issue Jan 13, 2021 · 3 comments

Comments

@piegamesde
Copy link

Reopening of #133.

I tried to use {? } blocks, but could not compiled.
{? } blocks seem to require parser::RuleResult rather than std::result::Result, and I don't know the arguments of parser::RuleResult::Matched.
Sorry, it was my confusion that I used return statement in a loop in {? } block.

IMO, this is a bug and not a user error. I'd expect to be able to use the full power of control flow, but I can't.

Workaround: wrap the code in a closure:

{?
    let dummy_closure = || {
        // Business logic in here, including control flow like the ?-operator
    };
    dummy_closure()
}
kevinmehall added a commit that referenced this issue Jan 18, 2021
Allow `return` and `?` in the code block (#246), and prevent statements
like `break`/`continue` from messing up enclosing loops.
kevinmehall added a commit that referenced this issue Jan 18, 2021
Allow `return` and `?` in the code block (#246), and prevent statements
like `break`/`continue` from messing up enclosing loops.
@kevinmehall
Copy link
Owner

In #248, I tested making the expansion wrap all user-provided code in an immediately-invoked closure ((||{ ... })()) and it seems to mostly work, except it causes a regression in the compile error message when the code block returns the wrong type. Rustc annotates the error on the whole macro rather than just the code block. This can probably be worked around by obtaining the source span of the block in the parser and applying it to the closure rather than the default call_site span.

I'm also slightly concerned about the closure causing type inference issues. Definitely worth further investigation, though.

@SpecificProtagonist
Copy link

Can the definition of RuleResult simply be changed to type RuleResult<T> = Result<(usize, T), Failed>;?

If for some reason (backwards compatibility?) this isn't possible, RuleResult could implement Try, although that would have to be gated under a feature because it's only available in nightly.

@kevinmehall
Copy link
Owner

RuleResult could be a Result, but that's a separate issue -- it's only intended to be in the public API for the traits for custom input types. It's only accidentally exposed to {? }.

The issue is that the code block ends up somewhere in the middle of a big function that parses a whole rule of the grammar. If you were toreturn from that function, the ParseResult would represent the result of the whole rule that contains the block, without trying subsequent / alternatives in the containing expression. If you returned Ok(), the position and value would become the result and continuation position of the whole rule.

If the code block is wrapped in a closure, ? and return would just return from the block, and parsing within the rule would continue.

kevinmehall added a commit that referenced this issue Feb 14, 2021
Allow `return` and `?` in the code block (#246), and prevent statements
like `break`/`continue` from messing up enclosing loops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants