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 all 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
122 changes: 97 additions & 25 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
type_ascription,
..
} = &**var_decl;
// Connect variable declaration node to its type ascription.
connect_type_id(engines, type_ascription.type_id, graph, entry_node)?;

// Connect variable declaration node to body expression.
let result = connect_expression(
engines,
Expand All @@ -533,6 +532,14 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
body.clone().span,
options,
);

if let Ok(ref vec) = result {
if !vec.is_empty() {
// Connect variable declaration node to its type ascription.
connect_type_id(engines, type_ascription.type_id, graph, entry_node)?;
}
}

// Insert variable only after connecting body.expressions
// This enables:
// let ptr = alloc::<u64>(0);
Expand Down Expand Up @@ -1219,7 +1226,22 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
)?;
}

let mut leaves = leaves.to_vec();
let mut args_diverge = false;
for (_name, arg) in arguments {
if type_engine
.get(arg.return_type)
.is_uninhabited(engines.te(), engines.de())
{
args_diverge = true;
}
}

let mut param_leaves = leaves.to_vec();
let mut leaves = if args_diverge {
vec![]
} else {
leaves.to_vec()
};

// if the parent node exists in this module, then add the monomorphized version
// to the graph.
Expand Down Expand Up @@ -1338,13 +1360,20 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
engines,
&arg.expression,
graph,
&current_leaf,
&param_leaves,
exit_node,
"arg eval",
tree_type,
span,
options,
)?;

if type_engine
.get(arg.return_type)
.is_uninhabited(engines.te(), engines.de())
{
param_leaves = vec![];
}
}
options.force_struct_fields_connection = force_struct_fields_connection;

Expand All @@ -1363,16 +1392,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 +1744,17 @@ 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)
.is_uninhabited(engines.te(), engines.de())
{
element_diverge = true
}
connect_expression(
engines,
&elem.expression,
Expand All @@ -1727,7 +1768,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,15 +1888,21 @@ 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(),
);
let mut leaves = vec![entry];

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(),
);
} else {
// As condition return type is NeverType we should not connect the remaining nodes to entry.
leaves = vec![];
}

// handle the condition of the loop
connect_expression(
engines,
Expand Down Expand Up @@ -2074,8 +2125,25 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
(node_idx, node_idx)
});

let mut is_variant_unreachable = false;
if let Some(instantiator) = contents {
if engines
.te()
.get(instantiator.return_type)
.is_uninhabited(engines.te(), engines.de())
{
is_variant_unreachable = true;
}
}

let leaves = if is_variant_unreachable {
vec![]
} else {
leaves.to_vec()
};

// Connects call path decl, useful for aliases.
connect_call_path_decl(engines, call_path_decl, graph, leaves)?;
connect_call_path_decl(engines, call_path_decl, graph, &leaves)?;

// insert organizational nodes for instantiation of enum
let enum_instantiation_entry_idx = graph.add_node("enum instantiation entry".into());
Expand All @@ -2084,7 +2152,7 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
// connect to declaration node itself to show that the declaration is used
graph.add_edge(enum_instantiation_entry_idx, decl_ix, "".into());
for leaf in leaves {
graph.add_edge(*leaf, enum_instantiation_entry_idx, "".into());
graph.add_edge(leaf, enum_instantiation_entry_idx, "".into());
}

// add edge from the entry of the enum instantiation to the body of the instantiation
Expand All @@ -2100,15 +2168,19 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
enum_decl.span.clone(),
options,
)?;

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());

Ok(vec![enum_instantiation_exit_idx])
if !is_variant_unreachable {
graph.add_edge(variant_index, enum_instantiation_exit_idx, "".into());
Ok(vec![enum_instantiation_exit_idx])
} else {
Ok(vec![])
}
}

/// Given a [ty::TyAstNode] that we know is not reached in the graph, construct a warning
Expand Down
6 changes: 5 additions & 1 deletion sway-core/src/control_flow_analysis/flow_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,11 @@ impl<'cfg> ControlFlowGraph<'cfg> {

pub(crate) fn get_node_from_decl(&self, cfg_node: &ControlFlowGraphNode) -> Option<NodeIndex> {
if let Some(ident) = cfg_node.get_decl_ident() {
self.decls.get(&ident.into()).cloned()
if !ident.span().is_dummy() {
self.decls.get(&ident.into()).cloned()
} else {
None
}
} else {
None
}
Expand Down
8 changes: 8 additions & 0 deletions sway-core/src/type_system/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,7 @@ impl TypeInfo {
let id_uninhabited = |id| type_engine.get(id).is_uninhabited(type_engine, decl_engine);

match self {
TypeInfo::Never => true,
TypeInfo::Enum(decl_ref) => decl_engine
.get_enum(decl_ref)
.variants
Expand All @@ -1028,6 +1029,13 @@ impl TypeInfo {
.iter()
.any(|field_type| id_uninhabited(field_type.type_id)),
TypeInfo::Array(elem_ty, length) => length.val() > 0 && id_uninhabited(elem_ty.type_id),
TypeInfo::Ptr(ty) => id_uninhabited(ty.type_id),
TypeInfo::Alias { name: _, ty } => id_uninhabited(ty.type_id),
TypeInfo::Slice(ty) => id_uninhabited(ty.type_id),
TypeInfo::Ref {
to_mutable_value: _,
referenced_type,
} => id_uninhabited(referenced_type.type_id),
_ => false,
}
}
Expand Down
4 changes: 4 additions & 0 deletions sway-types/src/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ impl Span {
pub fn line_col(&self) -> (LineCol, LineCol) {
(self.start_pos().line_col(), self.end_pos().line_col())
}

pub fn is_dummy(&self) -> bool {
self.eq(&DUMMY_SPAN)
}
}

impl fmt::Debug for Span {
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 @@ -299,6 +299,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 = 38
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 = 36
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 = 36
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
Loading
Loading