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

Parse subslice patterns #1509

Closed
wants to merge 1 commit into from

Conversation

tomjakubowski
Copy link

This attempt detects subslice patterns in the same place as range patterns;
when the right hand side of a .. in a potential range definitely isn't a pattern,
the parser stops and emits a SUBSLICE_PAT instead.

cc #1479

@tomjakubowski
Copy link
Author

tomjakubowski commented Jul 8, 2019

@matklad you said the adjustment would need to be in slice_pat but I couldn't figure that out.

In an example like let [a, b, c..], pattern_r() errored in the range pattern branch, on forms like let [a, b, c..] = [], because it would call atom_pat() on the ] token. I didn't see a way to work around this without changing pattern_r(). It was also easier to make more complicated subslice patterns like [ref a..] parse this way.

It's unfortunate that subslice patterns can be parsed outside of slice patterns. I think I can fix that by adding a bool flag to pattern()/pattern_r() signaling whether to parse subslice patterns. That seems a little gross, which often suggests there's a better design...

atom_pat(p, recovery_set);
m.complete(p, RANGE_PAT);
if subslice_patterns && dots == T![..] && !p.at_ts(PATTERN_FIRST) {
m.complete(p, SUBSLICE_PAT);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think this is still not exactly what we want, as it'll match [1..], which is wrong: we need to enfroce that lhs is identifier.

I think the best solution would be to inline pat_list into slice_pat and handle a.. specially, simliar to ..:

T![..] => p.bump(),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, I see pat_list is already inlined! So, we only need to pull haling of .. out of pattern_with_subslice_patterns into the while !p.at(EOF) && !p.at(T![']']) { loop

@tomjakubowski
Copy link
Author

tomjakubowski commented Jul 15, 2019

OK, this commit copies from atom_pat() to detect binding patterns (including ref mut box a) in slice_pat(). If .. is then eaten, a SUBSLICE_PAT token is emitted, otherwise just the regular binding pattern.

rustc does actually parse .. after some other kinds of patterns, like path patterns and certain literals, but I suspect it's not meant to: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=19d785643aba0341309d7c5b08098ad7

@tomjakubowski
Copy link
Author

tomjakubowski commented Jul 15, 2019

I think this won't parse macro patterns followed by .., like in let [m!(a).., b] = []. Maybe that case could be handled in the branch that calls pattern(): check if it returned a MACRO_CALL; if so, try and eat a .. and potentially emit a SUBSLICE_PAT.

@matklad
Copy link
Member

matklad commented Jul 15, 2019

Wow, looks like I don't really understand what this syntax means in the first place... I would expect that macros wouldn't be allowed to the left of ..... Asked a clarifying question in the original issue.

@matklad
Copy link
Member

matklad commented Jul 15, 2019

Ok, so after reading through several RFC, it looks like x.. is an old syntax which will be changed soon, so there's no real reason to support it. Rather, we should move in the direction of rust-lang/rfcs#2707, which just makes .. a pattern of its own.

Chasing unstable features is annoying :)

@Centril
Copy link
Contributor

Centril commented Jul 24, 2019

Ok, so after reading through several RFC, it looks like x.. is an old syntax which will be changed soon, so there's no real reason to support it. Rather, we should move in the direction of rust-lang/rfcs#2707, which just makes .. a pattern of its own.

The syntax x.. is removed and replaced with .. as a pattern in rust-lang/rust#62550.

@Centril
Copy link
Contributor

Centril commented Jul 28, 2019

UPDATE: The PR rust-lang/rust#62550 has been merged.

@matklad
Copy link
Member

matklad commented Oct 8, 2019

closing due to inactivity!

@matklad matklad closed this Oct 8, 2019
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

Successfully merging this pull request may close these issues.

3 participants