Skip to content

Allow pat macro matcher be followed by | #1336

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

Closed
nagisa opened this issue Oct 24, 2015 · 16 comments
Closed

Allow pat macro matcher be followed by | #1336

nagisa opened this issue Oct 24, 2015 · 16 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@nagisa
Copy link
Member

nagisa commented Oct 24, 2015

Since code like

match {
    pat | pat => …
}

is possible, there’s no problem if pat is followed by a |. It should be allowed in macro so e.g.

macro_rules! eat_token {
    ($s: ident, $($p: pat)|+) => {
        match $s.current.t {
            $($p)|+ => {
                try!($s.bump());
                Ok(())
            }
            _ => $s.unexpected()
        }
    }
}

//used like
eat_token!(self, TokenThis(_) | TokenThat(_))

is possible.

@nagisa nagisa changed the title Allow pat macro matched be followed by | Allow pat macro matcher be followed by | Oct 24, 2015
@petrochenkov
Copy link
Contributor

Workaround (stolen from https://github.com/SimonSapin/rust-std-candidates/blob/master/matches/lib.rs):

macro_rules! _tt_as_expr_hack {
    ($value:expr) => ($value)
}

macro_rules! eat_token {
    ($s: ident, $($p:tt)+) => {
        _tt_as_expr_hack! {
            match $s.current.t {
                $($p)+ => {
                    try!($s.bump());
                    Ok(())
                }
                _ => $s.unexpected()
            }
        }
    }
}

fn main() {
    eat_token!(self, TokenThis(_) | TokenThat(_) if condition)
}

@Stebalien
Copy link
Contributor

Also, |pat| {}.

@durka
Copy link
Contributor

durka commented Dec 3, 2015

Along these lines, it seems to me that:

  • | should be in FOLLOW(ty) because of closure syntax e.g. |x: i32| x + 1
  • ; should be in FOLLOW(ty) because of array syntax e.g. [i32; 42]

Is this right or do I have misconceptions about what FOLLOW really means?

@petrochenkov that workaround sort of works but (a) only if $($p:pat)|+ was going to be the end of the macro syntax, and (b) it doesn't let you separate the matches, i.e. to transform $($p:pat)|+ into $($p),+ or whatever. Both of these can also be solved but it requires recursion.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 3, 2015

@durka yes I agree; I think all changes suggested are covered by #1384

@durka
Copy link
Contributor

durka commented Dec 3, 2015

Great. Looking forward to that RFC :)

@huonw
Copy link
Member

huonw commented Dec 3, 2015

This was originally disallowed because we were thinking we might expand pat to include pat | pat, i.e. Some(_) | None is a single pattern, so something like let Some(_) | None = foo() is valid, although this may be killed by closures; at least without more tweaking/restricting the pattern grammar in closures.

(I guess this is more compelling with enum Foo { A(u8), B(u8) } and then let A(x) | B(x) = returns_a_foo(); would compile, and x would be in scope.)

@durka
Copy link
Contributor

durka commented Dec 3, 2015

Ah I see. Though, that specific case could be done by changing the grammar of let instead (conceptually, from let $p:pat = $e:expr to let $($p:pat)|+ = $e:expr).

@huonw
Copy link
Member

huonw commented Dec 3, 2015

Yes, the let case is, but the change is a natural generalisation of match, allowing | in all pattern locations without having to special case each one (i.e. including existing macros that use pat).

@durka
Copy link
Contributor

durka commented Dec 3, 2015

Right, makes sense. Seems like closures do kill it, unless the multi-pat syntax isn't allowed in closure/function arguments... but then that seems to cry out for another fragment specifier along the lines of moelarry...

@durka
Copy link
Contributor

durka commented Dec 3, 2015

Or if patterns were allowed to be enclosed in parentheses, that could save it.

@petrochenkov
Copy link
Contributor

@huonw

we were thinking we might expand pat to include pat | pat

That would be useful, I remember I needed this and certainly more than once. If it could be done by minor not-very-breaking-in-practice restrictions in other parts of grammar, it would totally worth it.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

Or if patterns were allowed to be enclosed in parentheses, that could save it.

Ah, that could make everyone happy... e.g. I would still be able to put | in the follow set for pat, in that case...

@durka
Copy link
Contributor

durka commented Dec 4, 2015

It still could create a breaking change -- if pat | pat becomes matchable by pat but not allowed in, say, closure arguments, then a macro that previously used a pat to construct a closure would no longer work on all inputs. It could be fixed by adding parentheses to the macro output (or whatever other workaround is devised) but it would still be breaking.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2015

@durka ah sorry I was not clear

What I meant was: if pat | pat were not a pat, but (pat | pat) was a pat, then that could be a smoother way forward

@durka
Copy link
Contributor

durka commented Dec 4, 2015

Ah gotcha.

On Fri, Dec 4, 2015 at 12:49 PM, Felix S Klock II [email protected]
wrote:

@durka https://github.com/durka ah sorry I was not clear

What I meant was: if pat | pat were not a pat, but (pat | pat) was a
pat, then that could be a smoother way forward


Reply to this email directly or view it on GitHub
#1336 (comment).

@pnkfelix
Copy link
Member

(this has been folded into RFC amendment #1384, which has been accepted.)

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

7 participants