-
Notifications
You must be signed in to change notification settings - Fork 182
gccrs: Add initial compilation support for IdentifierPattern's subpatterns #3814
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
gccrs: Add initial compilation support for IdentifierPattern's subpatterns #3814
Conversation
gcc/rust/ChangeLog: * backend/rust-compile-pattern.cc: Add CheckExpr compilation for IdentifierPattern with subpattern. * backend/rust-compile-pattern.h: Modify IdentifierPattern's visit func to support the above. * typecheck/rust-hir-type-check-pattern.cc: Add typechecking support for the changes above. Signed-off-by: Yap Zhi Heng <[email protected]>
gcc/rust/ChangeLog: * ast/rust-ast-collector.cc: Rename to_bind to subpattern. * ast/rust-ast-visitor.cc: Ditto. * ast/rust-pattern.cc: Ditto. * ast/rust-pattern.h: Ditto. * backend/rust-compile-pattern.cc: Ditto. * expand/rust-cfg-strip.cc: Ditto. * hir/rust-ast-lower-pattern.cc: Ditto. * hir/rust-hir-dump.cc: Ditto. * hir/tree/rust-hir-pattern.h: Ditto. * hir/tree/rust-hir.cc: Ditto. * typecheck/rust-hir-type-check-pattern.cc: Ditto. Signed-off-by: Yap Zhi Heng <[email protected]>
1f5fb1d
to
37a5e70
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.
good job @Polygonalr!! this PR looks really nice and the test cases are really good.
{ | ||
if (pattern.has_subpattern ()) | ||
{ | ||
check_expr = CompilePatternCheckExpr::Compile (pattern.get_subpattern (), | ||
match_scrutinee_expr, ctx); | ||
} | ||
else | ||
{ | ||
check_expr = boolean_true_node; | ||
} |
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.
well done!
void visit (HIR::IdentifierPattern &) override; | ||
|
||
// Always succeeds | ||
void visit (HIR::IdentifierPattern &) override | ||
{ | ||
check_expr = boolean_true_node; | ||
} |
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.
good job, this is always a good cleanup - we try prioritizing implementations in .cc files rather than .h files
{ | ||
TypeCheckPattern::Resolve (pattern.get_subpattern (), parent); | ||
} |
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.
GCC style is to avoid braces for one-line if
blocks, but this is a nitpick - feel free to disregard
{ | |
TypeCheckPattern::Resolve (pattern.get_subpattern (), parent); | |
} | |
TypeCheckPattern::Resolve (pattern.get_subpattern (), parent); |
Looks like you might be missing some changes to |
Adds initial support for compilation of IdentifierPattern's subpattern. For now, tested and works with simple
LiteralPattern
subpatterns; more complex subpatterns require further testing.make check-rust
passes locallyclang-format
gcc/testsuite/rust/
Reviewers: 1st commit contains the actual changes, 2nd commit only contains variable renaming.