Skip to content

disallow | from the follow-set of pat2021 #84138

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
nikomatsakis opened this issue Apr 12, 2021 · 7 comments · Fixed by #84185
Closed

disallow | from the follow-set of pat2021 #84138

nikomatsakis opened this issue Apr 12, 2021 · 7 comments · Fixed by #84185
Assignees
Labels
T-lang Relevant to the language team

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 12, 2021

In Edition 2021, the following should error:

macro_rules! foo {
    ($x:pat | $y:pat) => { }
}

fn main() { }

for the same reason that the following already errors:

macro_rules! foo {
    ($x:expr | $y:expr) => { }
}

fn main() { }

cc @rust-lang/project-edition-2021

@nikomatsakis nikomatsakis added the T-lang Relevant to the language team label Apr 12, 2021
@nikomatsakis
Copy link
Contributor Author

@hi-rustin I was wondering if you were interested in fixing this as a follow-up to #83468 -- I don't think you included it in that PR, right?

@0xPoe
Copy link
Member

0xPoe commented Apr 13, 2021

@rustbot claim

@0xPoe
Copy link
Member

0xPoe commented Apr 14, 2021

I think the problem is here:

NonterminalKind::Stmt | NonterminalKind::Expr => {
, but if we allow |, then

macro_rules! foo {
    ($x:expr |) => { }
} 

will also be allowed. This does not seem right. But in the current function we can't yet tell what token comes next in |. Do you have any suggestions?

@nikomatsakis
Copy link
Contributor Author

Actually, the code may already be correct? It appears that | is not allowed for pat2021, which is the goal:

NonterminalKind::Pat2021 { .. } => {
const TOKENS: &[&str] = &["`=>`", "`,`", "`=`", "`if`", "`in`"];
match tok {
TokenTree::Token(token) => match token.kind {
FatArrow | Comma | Eq => IsInFollow::Yes,
Ident(name, false) if name == kw::If || name == kw::In => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
},
_ => IsInFollow::No(TOKENS),
}
}

Note that | is allowed for pat2015. Maybe we just need a test -- or maybe there already is one!

@0xPoe
Copy link
Member

0xPoe commented Apr 14, 2021

Actually, the code may already be correct? It appears that | is not allowed for pat2021, which is the goal:

NonterminalKind::Pat2021 { .. } => {
const TOKENS: &[&str] = &["`=>`", "`,`", "`=`", "`if`", "`in`"];
match tok {
TokenTree::Token(token) => match token.kind {
FatArrow | Comma | Eq => IsInFollow::Yes,
Ident(name, false) if name == kw::If || name == kw::In => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
},
_ => IsInFollow::No(TOKENS),
}
}

Note that | is allowed for pat2015. Maybe we just need a test -- or maybe there already is one!

Yes this part of the logic I have added, but my question is about ($x:expr | $y:expr) => { }. I will add an additional test for pat2021.

@nikomatsakis
Copy link
Contributor Author

@hi-rustin I expect $x:expr | $y:expr to be an error -- it is, right?

@0xPoe
Copy link
Member

0xPoe commented Apr 14, 2021

@hi-rustin I expect $x:expr | $y:expr to be an error -- it is, right?

Yes. I see what you mean. I misunderstood your description in the issue.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 14, 2021
…omatsakis

add more pat2021 tests

close rust-lang#84138

r? `@nikomatsakis`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 14, 2021
…omatsakis

add more pat2021 tests

close rust-lang#84138

r? ``@nikomatsakis``
@bors bors closed this as completed in 13effcb Apr 15, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
T-lang Relevant to the language team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants