Skip to content

Commit

Permalink
DCA now detects dead code when all branches return explicitly. (FuelL…
Browse files Browse the repository at this point in the history
…abs#4169)

Fixing `If` was straightforward. But that didn't automatically the fix
`match` scenario (see test case). Turns out that `match` always has a
covering "ImplicitReturn" expression, and with implicit returns in DCA,
we add an edge from the beginning to the end, which I couldn't figure
out why. Just removing that got it working. I request the reviewers to
validate this part of the change carefully.

Closes FuelLabs#2313

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] 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.
  • Loading branch information
vaivaswatha authored Feb 24, 2023
1 parent 9346bf4 commit 253d6af
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 7 deletions.
11 changes: 4 additions & 7 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,6 @@ fn connect_node<'eng: 'cfg, 'cfg>(
options,
)?;

for leaf in return_contents.clone() {
graph.add_edge(this_index, leaf, "".into());
}
// connect return to the exit node
if let Some(exit_node) = exit_node {
graph.add_edge(this_index, exit_node, "return".into());
Expand Down Expand Up @@ -1040,7 +1037,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
engines,
&then.expression,
graph,
leaves,
&condition_expr,
exit_node,
"then branch",
tree_type,
Expand All @@ -1053,18 +1050,18 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
engines,
&else_expr.expression,
graph,
leaves,
&condition_expr,
exit_node,
"else branch",
tree_type,
else_expr.clone().span,
options,
)?
} else {
vec![]
condition_expr
};

Ok([condition_expr, then_expr, else_expr].concat())
Ok([then_expr, else_expr].concat())
}
CodeBlock(a @ ty::TyCodeBlock { .. }) => {
connect_code_block(engines, a, graph, leaves, exit_node, tree_type, options)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[[package]]
name = 'all_paths_return'
source = 'member'
dependencies = ['core']

[[package]]
name = 'core'
source = 'path+from-root-F8249D11C4071B26'
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "all_paths_return"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
script;

fn if_f() -> u64 {
if true {
return 0;
} else {
return 1;
}
// should trigger a warning
return 2;
}

fn match_f() -> u64 {
match 10 {
1 => return 8,
_ => return 3,
}
// should trigger a warning
return 21;
}

fn main() -> u64 {
if true {
return if_f();
} else {
return match_f();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
category = "compile"

# check: $()return 2;
# nextln: $()This code is unreachable.

# check: $()return 21;
# nextln: $()This code is unreachable.

0 comments on commit 253d6af

Please sign in to comment.