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

macro parser should back out of failed nonterminal parsing #3232

Closed
paulstansifer opened this Issue Aug 20, 2012 · 11 comments

Comments

Projects
None yet
8 participants
@paulstansifer
Copy link
Contributor

paulstansifer commented Aug 20, 2012

Currently, the following macro argument grammar will not successfully parse anything, complaining of a "local ambiguity":

 $( $t:ty )* #

The problem is that if it were to use the Rust parser to try to parse a type and fail, the whole task would be killed. This is easy to fix after that problem is resolved. There are two options:

  1. Break parsing into a separate task, and see whether it survives. However, parse.rs requires a parse_sess, which is mutable state, and can't be shared. So the second option is probably more doable:
  2. Allow the parser to safely return instead of killing the task. This works much like the Error monad. Ordinarily, this would require a lot of tedious wrapping and unwrapping, but we have a macro system! It should be relatively easy to make a notation like Haskell's do, and the parser would be able to stay in the standard imperative style.
@eholk

This comment has been minimized.

Copy link
Contributor

eholk commented Aug 22, 2012

+1

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Aug 22, 2012

Is there an option (3)---require that the grammar be LALR(k) or something like that? Or does that fundamentally not work?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Aug 22, 2012

That said: we should do (2) anyway, because it's lame that we cannot recover what-so-ever from parse failures.

@paulstansifer

This comment has been minimized.

Copy link
Contributor Author

paulstansifer commented Aug 22, 2012

The macro parser doesn't do any lookahead at all, and LR(0) is probably too
restrictive. So it's probably not a good idea to build in such a
restriction.

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Mar 22, 2013

non-critical for 0.6, de-milestoning

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Apr 25, 2013

nominating for backwards-compatible milestone. though if we agree to do (2) here maybe we can push this to production ready.

@catamorphism

This comment has been minimized.

Copy link
Contributor

catamorphism commented Jul 15, 2013

Adding nominated label, since I assume @graydon intended to nominate it to change the milestone.

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Jul 18, 2013

accepted for production-ready milestone

@vadimcn

This comment has been minimized.

Copy link
Contributor

vadimcn commented Sep 20, 2013

Option (4): Break parsing into a separate task and pass parse_sess to it unsafely.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 16, 2014

`accepted for P-low.

@paulstansifer paulstansifer self-assigned this May 11, 2014

@paulstansifer paulstansifer removed their assignment Jun 16, 2014

vadimcn added a commit to vadimcn/rust that referenced this issue Oct 7, 2014

Fix the most egregious instances of "local ambiguity: multiple parsin…
…g options..." error in macros, which often occurs when trying to match parts of Rust syntax.

For example, this matcher: `fn $name:ident( $($param:ident : $pty:ty),* )` would fail when parsing `fn foo()`, because macro parser wouldn't realize that an ident cannot start with `)`.

This resolves rust-lang#5902, and at least partially mitigates rust-lang#9364 and rust-lang#3232.

vadimcn added a commit to vadimcn/rust that referenced this issue Oct 7, 2014

Fix the most egregious instances of "local ambiguity: multiple parsin…
…g options..." error in macros, which often occurs when trying to match parts of Rust syntax.

For example, this matcher: `fn $name:ident( $($param:ident : $pty:ty),* )` would fail when parsing `fn foo()`, because macro parser wouldn't realize that an ident cannot start with `)`.

This resolves rust-lang#5902, and at least partially mitigates rust-lang#9364 and rust-lang#3232.
@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jun 29, 2015

Given that this is mentioned in rust-lang/rfcs#440, I'm going to be closing as 'not a bug' at this time, anyway. If we decide to improve macros, this is something that'd be nice to fix, for sure, but that RFC issue will be where that happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.