-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix: Perform scope discovery on match cases too #639
Conversation
rebase? |
889a0b0
to
e442db5
Compare
e442db5
to
00a5a69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I don't mind if the suggestion is implemented or not.
if let Some(name) = &spread.name { | ||
if let Some(member_id) = | ||
self.ast_info().stack_members().get_data_by_node(name.ast_ref().id()) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use a let_chain
here:
if let Some(name) = &spread.name { | |
if let Some(member_id) = | |
self.ast_info().stack_members().get_data_by_node(name.ast_ref().id()) | |
{ | |
if let Some(name) = &spread.name && let Some(member_id) = | |
self.ast_info().stack_members().get_data_by_node(name.ast_ref().id()) | |
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like let chains cause they are not yet supported by rustfmt rust-lang/rustfmt#5203. So I'll just keep it like this for now..
The scope discovery and symbol resolution stages did not handle match cases before. This PR implements this. Match cases are similar to declarations in that bindings inside the pattern are added to the child scope. Consequently, match cases create their own scopes.
Closes #637.