Add basic struct pattern rebinding #3741
Conversation
gcc/rust/ChangeLog: * backend/rust-compile-pattern.h: Split struct pattern compilation into three functions. * backend/rust-compile-pattern.cc: Implement them.
Allow matching on a struct instance and rebinding its fields to new names:
match X {
Foo {
field0: new_name0,
field1: new_name1,
} => {
do_something(new_name0, new_name1);
},
}
This will enable us to finish derives for PartialEq and PartialOrd but
isn't a complete implementation of these patterns yet.
gcc/rust/ChangeLog:
* backend/rust-compile-pattern.cc (CompilePatternBindings::make_struct_access):
New function.
(CompilePatternBindings::visit): Properly implement patterns mentioned above
and call make_struct_accesss.
* backend/rust-compile-pattern.h: New declaration.
gcc/testsuite/ChangeLog:
* rust/execute/torture/struct_pattern1.rs: New test.
gcc/rust/ChangeLog: * backend/rust-compile-pattern.cc (CompilePatternCheckExpr::visit): Remove old invalid checks.
CohenArthur
requested review from
philberty,
P-E-P and
powerboat9
and removed request for
P-E-P
It's been a while, but I thought I already did that Edit: Yeah, |
| int variant_index); | ||
| void handle_struct_pattern_ident_pat (HIR::StructPatternField &pat); | ||
| void handle_struct_pattern_tuple_pat (HIR::StructPatternField &pat); | ||
|
|
There was a problem hiding this comment.
These should probably be private
| ident.get_locus ()); | ||
| case HIR::StructPatternField::ItemType::IDENT_PAT: | ||
| // we only support rebinding patterns at the moment, which always | ||
| // match - so do nothing |
There was a problem hiding this comment.
What's wrong with the old checks?
There was a problem hiding this comment.
they caused an invalid call to struct_field_expression which I didn't want to explore. since when compiling struct pattern matching with StructPatternFieldIdentPat (so match x { Foo { field: new_name } => {} } we would always reach a rust_unreachable(), I think this code was actually never ever reached or executed and was just wrong
There was a problem hiding this comment.
Yeah, I think this was meant to handle struct-like enum items, but was probably broken by 806166d. Is there any chance you could fix it, rather than remove it? I could explain any parts of the code you have questions about, or fix it myself and give you the patch.
There was a problem hiding this comment.
but maybe I'm wrong? happy to change it and try to fix it instead if you think this was correct
There was a problem hiding this comment.
I think the code is close enough to right that getting it to work should be pretty simple -- check out where match_scrutinee_expr is being modified in the if statement above, you may have to remove that for struct_field_expression to work.
There was a problem hiding this comment.
sounds good, I'll take a look then :) didn't realize it had been modified recently, my bad! I thought I was just hitting an uncharted part of the codebase lol
There was a problem hiding this comment.
Not all that recently -- I'm pretty sure I wrote it back in late 2023. You are probably right that it was never actually being executed, though I think it should function with a few tweaks
This PR adds support for basic struct ident patterns, specifically in the case of rebinding struct fields to new names:
This is required for finish support for
derive(PartialEq)andderive(PartialOrd)