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

Improves DCA warnings after divergent expressions. #5726

Merged
merged 14 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 48 additions & 17 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
)?;
}

let mut args_diverge = false;
// we evaluate every one of the function arguments
for (_name, arg) in arguments {
let span = arg.span.clone();
Expand All @@ -1345,6 +1346,10 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
span,
options,
)?;

if type_engine.get(arg.return_type).contains_never(engines) {
args_diverge = true;
}
}
options.force_struct_fields_connection = force_struct_fields_connection;

Expand All @@ -1363,16 +1368,20 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
}
}
}
// the exit points get connected to an exit node for the application
if !is_external {
if let Some(exit_node) = exit_node {
graph.add_edge(fn_exit_point, exit_node, "".into());
Ok(vec![exit_node])
if args_diverge {
Ok(vec![])
} else {
// the exit points get connected to an exit node for the application
if !is_external {
if let Some(exit_node) = exit_node {
graph.add_edge(fn_exit_point, exit_node, "".into());
Ok(vec![exit_node])
} else {
Ok(vec![fn_exit_point])
}
} else {
Ok(vec![fn_exit_point])
Ok(vec![fn_entrypoint])
}
} else {
Ok(vec![fn_entrypoint])
}
}
LazyOperator { lhs, rhs, .. } => {
Expand Down Expand Up @@ -1711,9 +1720,14 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
elem_type: _,
contents,
} => {
let mut element_diverge = false;
let nodes = contents
.iter()
.map(|elem| {
if !element_diverge && type_engine.get(elem.return_type).contains_never(engines)
{
element_diverge = true
}
connect_expression(
engines,
&elem.expression,
Expand All @@ -1727,7 +1741,11 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
)
})
.collect::<Result<Vec<_>, _>>()?;
Ok(nodes.concat())
if element_diverge {
Ok(vec![])
} else {
Ok(nodes.concat())
}
}
ArrayIndex { prefix, index } => {
let prefix_idx = connect_expression(
Expand Down Expand Up @@ -1843,13 +1861,15 @@ fn connect_expression<'eng: 'cfg, 'cfg>(

let while_loop_exit = graph.add_node("while loop exit".to_string().into());

// it is possible for a whole while loop to be skipped so add edge from
// beginning of while loop straight to exit
graph.add_edge(
entry,
while_loop_exit,
"condition is initially false".into(),
);
if !matches!(*type_engine.get(condition.return_type), TypeInfo::Never) {
// it is possible for a whole while loop to be skipped so add edge from
// beginning of while loop straight to exit
graph.add_edge(
entry,
while_loop_exit,
"condition is initially false".into(),
);
}
let mut leaves = vec![entry];

// handle the condition of the loop
Expand Down Expand Up @@ -2087,6 +2107,7 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
graph.add_edge(*leaf, enum_instantiation_entry_idx, "".into());
}

let mut is_variant_unreachable = false;
// add edge from the entry of the enum instantiation to the body of the instantiation
if let Some(instantiator) = contents {
let instantiator_contents = connect_expression(
Expand All @@ -2100,13 +2121,23 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
enum_decl.span.clone(),
options,
)?;
if engines
.te()
.get(instantiator.return_type)
.contains_never(engines)
{
is_variant_unreachable = true;
}

for leaf in instantiator_contents {
graph.add_edge(leaf, enum_instantiation_exit_idx, "".into());
}
}

graph.add_edge(decl_ix, variant_index, "".into());
graph.add_edge(variant_index, enum_instantiation_exit_idx, "".into());
if !is_variant_unreachable {
graph.add_edge(variant_index, enum_instantiation_exit_idx, "".into());
}

Ok(vec![enum_instantiation_exit_idx])
}
Expand Down
37 changes: 37 additions & 0 deletions sway-core/src/type_system/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,43 @@ impl TypeInfo {
matches!(self, TypeInfo::Tuple(_))
}

pub fn contains_never(&self, engines: &Engines) -> bool {
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
match self {
TypeInfo::Never => true,
TypeInfo::Tuple(type_arguments) => type_arguments
.iter()
.any(|t| engines.te().get(t.type_id).contains_never(engines)),
TypeInfo::Array(type_argument, _) => engines
.te()
.get(type_argument.type_id)
.contains_never(engines),
TypeInfo::Unknown
| TypeInfo::UnknownGeneric { .. }
| TypeInfo::Placeholder(_)
| TypeInfo::TypeParam(_)
| TypeInfo::StringSlice
| TypeInfo::StringArray(_)
| TypeInfo::UnsignedInteger(_)
| TypeInfo::Enum(_)
| TypeInfo::Struct(_)
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
| TypeInfo::Boolean
| TypeInfo::ContractCaller { .. }
| TypeInfo::Custom { .. }
| TypeInfo::B256
| TypeInfo::Numeric
| TypeInfo::Contract
| TypeInfo::ErrorRecovery(_)
| TypeInfo::Storage { .. }
| TypeInfo::RawUntypedPtr
| TypeInfo::RawUntypedSlice
| TypeInfo::Ptr(_)
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
| TypeInfo::Slice(_)
| TypeInfo::Alias { .. }
| TypeInfo::TraitType { .. }
| TypeInfo::Ref { .. } => false,
}
}

pub(crate) fn apply_type_arguments(
self,
handler: &Handler,
Expand Down
2 changes: 2 additions & 0 deletions test/src/e2e_vm_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ impl TestContext {
run_and_capture_output(|| harness::compile_to_bytes(&name, &run_config)).await;
*output = out;

check_file_checker(checker, &name, output)?;

let compiled = result?;

let compiled = match compiled {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn in_length_2_array_first() -> u64 {
fn in_length_2_array_second() -> u64 {
let _ = [0, return];

145 // TODO: Missing unreachable warning
145
}

fn in_tuple() -> u64 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "run"
expected_result = { action = "return", value = 8193 }
validate_abi = true
expected_warnings = 26
expected_warnings = 33
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "run"
expected_result = { action = "return", value = 8193 }
validate_abi = true
expected_warnings = 25
expected_warnings = 31
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "run"
expected_result = { action = "return", value = 42 }
validate_abi = true
expected_warnings = 27
expected_warnings = 31
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,3 @@ category = "run"
expected_result = { action = "return", value = 42 }
validate_abi = true

# check: let _a: Result<u32, u8> = Ok(5);
# nextln: $()This cast, from integer type of width sixty four to integer type of width thirty two, will lose precision.

# check: let _b: MyStruct<u32> = MyStruct{ a:5 };
# nextln: $()This cast, from integer type of width sixty four to integer type of width thirty two, will lose precision.

expected_warnings = 2
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,34 @@ pub enum Enum_multivariant {
fn in_init() -> u64 {
let _ = return 42;

45
045
}

fn in_array() -> u64 {
let _ = [return 42, return 43];

145
1450
}

// Arrays of length 1 are treated differently
fn in_length_1_array() -> u64 {
let _ = [return 42];

145
1451
}

// The first element of an array is treated differently
fn in_length_2_array_first() -> u64 {
let _ = [return 42, 0];

145
1452
}

// The first element of an array is treated differently
fn in_length_2_array_second() -> u64 {
let _ = [0, return 42];

145 // TODO: Missing unreachable warning
1453
}

fn in_tuple() -> u64 {
Expand Down Expand Up @@ -90,19 +90,19 @@ fn in_while_condition() -> u64 {
break;
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
};

745 // TODO: Missing unreachable warning
745
}

fn in_enum() -> u64 {
let _ = Enum::A((return 42, return 43));

845 // TODO: Missing unreachable warning
845
}

fn in_enum_multivariant() -> u64 {
let _ = Enum_multivariant::B((return 42, return 43));
esdrubal marked this conversation as resolved.
Show resolved Hide resolved

945 // TODO: Missing unreachable warning
945
}

fn helper_fun(x : u64, y : u64) -> u64 {
Expand All @@ -112,7 +112,7 @@ fn helper_fun(x : u64, y : u64) -> u64 {
fn in_fun_arg() -> u64 {
let _ = helper_fun(return 42, return 43);

1045 // TODO: Missing unreachable warning
1045
}

fn in_lazy_and() -> u64 {
Expand All @@ -128,11 +128,11 @@ fn in_lazy_or() -> u64 {
}

fn in_match_scrutinee() -> u64 {
match return 42 {
let _ = match return 42 {
_ => 5411,
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
}
};

1145
1345
}


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,67 @@
category = "run"
expected_result = { action = "return", value = 8193 }
validate_abi = true
expected_warnings = 17
expected_warnings = 22

#check: $()045
#nextln: $()This code is unreachable.

#check: $()1450
#nextln: $()This code is unreachable.

#check: $()1451
#nextln: $()This code is unreachable.

#check: $()1452
#nextln: $()This code is unreachable.

#check: $()1453
#nextln: $()This code is unreachable.

#check: $()245
#nextln: $()This code is unreachable.

#check: $()345
#nextln: $()This code is unreachable.

#check: $()445
#nextln: $()This code is unreachable.

#check: $()545
#nextln: $()This code is unreachable.

#check: $()543
#nextln: $()This code is unreachable.

#check: $()345
#nextln: $()This code is unreachable.

#check: $()645
#nextln: $()This code is unreachable.

# check: $()745
# nextln: $()This code is unreachable.

#check: $()845
#nextln: $()This code is unreachable.

#check: $()945
#nextln: $()This code is unreachable.

#check: $()1045
#nextln: $()This code is unreachable.

#check: $()1145
#nextln: $()This code is unreachable.

#check: $()1245
#nextln: $()This code is unreachable.

#check: $()let _ = match return 42 {
#check: $()};
#nextln: $()This code is unreachable.
#nextln: $()
#nextln: $()1345

#check: $()1345
#nextln: $()This code is unreachable.
Loading