Skip to content
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

Abort control flow upon a return, RET, or RVRT. #964

Merged
merged 15 commits into from
Mar 22, 2022
Merged

Conversation

sezna
Copy link
Contributor

@sezna sezna commented Mar 18, 2022

This should unblock the final hard blocker for swayswap.

What does this do?

In Rust, if a branch aborts control flow, then its return type doesn't matter. Here is an example.

We want this behavior so we can implement .unwrap() -- panicking if there is no value present in an Option requires that we don't type check the else branch in case of a deterministic abort.

How does this do?

This PR introduces a .deterministically_aborts() method on AST nodes, expressions, and code blocks which returns a true if, in its control flow, there is a RVRT, RET, or return that is impossible to avoid. We then special case some type checking in branched code around that, as well as take it into consideration when type checking expressions against their annotations.

Other things

While implementing this, I found a bug in return statement type checking -- we were not type checking returns that are on the RHS of a variable declaration. For example, the below compiles when it should not.

fn main() -> u64 {
  let x = return true;
}

This PR includes a fix for that.

Closes #308

@sezna sezna added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Mar 18, 2022
@sezna sezna self-assigned this Mar 18, 2022
@otrho
Copy link
Contributor

otrho commented Mar 20, 2022

LGTM otherwise.

Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflicts. Could you move the testcase into the directory structure of #965?

I've tested locally and can confirm that old-compiler does not compile the testcase and new-compiler does.

Alexander Hansen and others added 2 commits March 21, 2022 16:21
@sezna sezna requested a review from adlerjohn March 21, 2022 23:26
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will defer to the actual compiler devs.

@sezna sezna merged commit 904ce24 into master Mar 22, 2022
@sezna sezna deleted the sezna/abort-control-flow branch March 22, 2022 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Abort control flow in control flow analysis/dead code analysis when std::chain::revert() is called
3 participants