Rust-GCC / gccrs

GCC Front-End for Rust
https://rust-gcc.github.io/
GNU General Public License v2.0
2.4k stars 155 forks source link

Lower WhileLoopExpr to LoopExpr #253

Open philberty opened 3 years ago

philberty commented 3 years ago

HIR lowers while loops to loop expr with a conditional break expression. We should do the same. There is logic to make sure breaks with value cannot occur inside while loop expressions which will need to be moved to a higher pass (possibly as part of lowering).

philberty commented 3 years ago

The starting point in this project is to remove the WhileLoopExpr from the rust/hir trees.

YizhePKU commented 3 years ago

According to what rustc does, the transformation is from

'label: while $cond $body

to

'label: loop {
    match drop_temps { $cond } {
        true => $body,
        _ => break,
    }
}

$cond needs to be evaluated inside the loop to allow breaking from $cond(This doesn't compile yet, issue #266). Any temporary values created within $cond needs to be dropped immediately, before entering the body of the loop.

YizhePKU commented 3 years ago

Currently, the "cannot break with value inside while loop" check is done as part of type-checking, which is done on the HIR(rust-hir-type-check-expr.h:827). It uses a single boolean inside_loop as context(which is kinda hacky).

To move that logic into the AST-to-HIR-lowering process, we also needs that context, similar to the LoweringContext in rustc. In particular we need to keep track of loop_scopes.

It might be a good idea to reorganize the code a bit. Currently the lowering function is scattered among many classes as static methods, such as ASTLoweringBase::lower_lifetime, ASTLoweringExpr::translate, ASTLoweringBlock::translate, etc. We could collect these methods info a new LoweringContext as methods.

philberty commented 3 years ago

You are on the right track, there are reasons the code is split up about the place. It is to keep the class hierarchy correct like ExprWithBlock and ExprWithoutBlock need to be recursive. If you want to reorganise that code that should be a separate PR and a separate issue.

Loop Scopes is a bigger thing to implement and since this style of lowering requires pattern matching I used a flag for now. There is logic to detect break with value inside the WhileLoopExpr that will need to be moved higher also.

Since this lowering depends on pattern matching it is why I implemented it the way it is currently. I am marking this with the control flow 2 milestone which is all about pattern matching. I also think it is not possible to start working on this without doing #190 first.

philberty commented 3 years ago

Removed good-first-pr for now until #190 is done.