Skip to content

Commit

Permalink
Introduce TerminatorValue to force handling of divergence (#5496)
Browse files Browse the repository at this point in the history
## Description

This PR refactors how `Value` is used during IR conversion. The PR
introduces `TerminatorValue` which forces the converter to handle
diverging expressions properly. There were checks for divergence
scattered around the code before, but it's now been made more systematic
and thus more robust.

Additionally, this change allows for early returns from some functions.
For instance, if the condition of a `while` diverges, then we don't need
to generate code for the body of the loop. In fact, if an expression
does diverge then the algorithm assumes that a terminator value (e.g., a
`branch` or a `return`) is returned, but this was not always the case,
and led to a number of spurious checks that probably didn't quite work.

The heavy lifting is done by the macro
`return_on_termination_or_extract`, which acts as destructor of
`TerminatorValue`s.

Most changes are straightforward, but do review the functions
`compile_lazy_op`, `compile_fn_call`, `compile_if`,
`compile_while_loop`, `compile_var_decl` and `compile_array_expr` extra
carefully, as they contain changes to the conversion algorithm. Mostly
the changes are small, but they need an extra pair of eyes. In a few
cases the algorithm was and still is a bit convoluted, but that's
because the typechecker unhelpfully assigns `Unknown` instead of `Never`
to diverging expressions, so the first sub-expression needs to be
compiled and divergence checked before we can generate IR for anything
that relies on the type of the expression.

Finally, I have taken the liberty of removing `Value::is_diverging()`
and using its alias `Value::is_terminator()` everywhere. The concept of
divergence lives in the source language, whereas the concept of
terminators lives in the IR, so `is_terminator` seems to be more
appropriate at this stage of the compilation.

Fixes #2051. 

Partial fix of #5438 (the call to `deterministically_aborts()` is
removed).

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Igor Rončević <[email protected]>
  • Loading branch information
jjcnn and ironcev authored Jan 29, 2024
1 parent 059b057 commit 61279a7
Show file tree
Hide file tree
Showing 5 changed files with 608 additions and 459 deletions.
2 changes: 1 addition & 1 deletion sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ fn compile_fn(
logged_types_map,
messages_types_map,
);
let mut ret_val = compiler.compile_code_block(context, md_mgr, body)?;
let mut ret_val = compiler.compile_code_block_to_value(context, md_mgr, body)?;

// Special case: sometimes the returned value at the end of the function block is hacked
// together and is invalid. This can happen with diverging control flow or with implicit
Expand Down
Loading

0 comments on commit 61279a7

Please sign in to comment.