2688 labelled block #2689
Merged
2688 labelled block #2689
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
gcc/rust/ChangeLog: * ast/rust-ast-builder.cc (AstBuilder::block): Add label arg to constructor call. * ast/rust-expr.h (class LoopLabel): Move before BlockExpr. (class BlockExpr): Add LoopLabel member. * expand/rust-derive-clone.cc (DeriveClone::clone_fn): Add label arg to constructor call. * parse/rust-parse-impl.h (Parser::parse_block_expr): Add label parameter. (Parser::parse_labelled_loop_expr): Add label arg to constructor call. * parse/rust-parse.h: Add label arg to constructor call. Signed-off-by: Jakub Dupak <[email protected]>
gcc/rust/ChangeLog: * hir/rust-ast-lower.cc (ASTLoweringBlock::visit): Call loop lowering and add it to constr. * hir/tree/rust-hir-expr.h (class LoopLabel): Move before BlockExpr and add to BlockExpr.
gcc/rust/ChangeLog: * resolve/rust-ast-resolve-expr.cc (ResolveExpr::visit): Resolve using loop logic. Signed-off-by: Jakub Dupak <[email protected]>
gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): Add loop ctx. Signed-off-by: Jakub Dupak <[email protected]>
gcc/rust/ChangeLog: * backend/rust-compile-expr.cc (CompileExpr::visit): Bail on labelled block. Signed-off-by: Jakub Dupak <[email protected]>
jdupak
force-pushed
the
2688-labeled-block
branch
from
2e226d6 to
9901240
Compare
P-E-P
approved these changes
Oct 20, 2023
| @@ -2121,6 +2149,7 @@ class BlockExpr : public ExprWithBlock, public WithInnerAttrs | |||
| std::vector<std::unique_ptr<Stmt> > statements; | |||
| std::unique_ptr<Expr> expr; | |||
| bool tail_reachable; | |||
| LoopLabel label; | |||
There was a problem hiding this comment.
Do we want to use tl::optional<LoopLabel> instead of label.is_error() ? This would help enforce a label exists.
There was a problem hiding this comment.
Sounds good to me, but probably not in this PR - the same logic is used for loops (was taken from there) and it would make more sense to change it everywhere at once, otherwise it is more confusing. Agree @P-E-P ?
There was a problem hiding this comment.
You're right. Let's merge this then 👍
CohenArthur
approved these changes
Oct 20, 2023
| new BlockExpr (std::move (stmts), std::move (tail_expr), {}, {}, loc, loc)); | ||
| return std::unique_ptr<Expr> (new BlockExpr (std::move (stmts), | ||
| std::move (tail_expr), {}, {}, | ||
| LoopLabel::error (), loc, loc)); |
There was a problem hiding this comment.
from an API point of view, I'm not sure if LoopLabel::error here is very nice. something like empty would probably be better? sorry for nitpicking haha
|
|
||
| location_t get_locus () const { return locus; } | ||
|
|
||
| Lifetime &get_lifetime () { return label; } |
There was a problem hiding this comment.
Suggested change
| Lifetime &get_lifetime () { return label; } | |
| Lifetime &get_label () { return label; } |
or
Suggested change
| Lifetime &get_lifetime () { return label; } | |
| Lifetime &get_name () { return label; } |
|
|
||
| Analysis::NodeMapping &get_mappings () { return mappings; } | ||
|
|
||
| Lifetime &get_lifetime () { return label; } |
| {} | ||
|
|
||
| // Returns whether the LoopLabel is in an error state. | ||
| bool is_error () const { return label.is_error (); } |
There was a problem hiding this comment.
same here, should be is_empty() IMO
| != AST::Lifetime::LifetimeType::NAMED) | ||
| { | ||
| rust_error_at (label.get_locus (), | ||
| "Labels must be a named lifetime value"); |
There was a problem hiding this comment.
Suggested change
| "Labels must be a named lifetime value"); | |
| "labels must be a named lifetime value"); |
for consistency :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds suport for labelled blocks up to compile (where it bails). Resolves #2688