Skip to content

Commit

Permalink
Fixes DCA warning in impl block instead of method. (FuelLabs#4326)
Browse files Browse the repository at this point in the history
## Description

When all of the impl methods are unused the whole impl block was marked
as unused.

Now the compiler shows a DCA warning for the unused methods instead of a
single warning for the impl block.

Closes FuelLabs#4311

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] 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.
  • Loading branch information
esdrubal authored Mar 29, 2023
1 parent adc5c1a commit 6f5cca5
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 33 deletions.
71 changes: 41 additions & 30 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,43 @@ impl<'cfg> ControlFlowGraph<'cfg> {
};

let is_alive_check = |n: &NodeIndex| {
if let ControlFlowGraphNode::ProgramNode {
node:
ty::TyAstNode {
content: ty::TyAstNodeContent::Declaration(ty::TyDecl::VariableDecl { .. }),
..
},
..
} = &self.graph[*n]
{
// Consider variables declarations alive when count is greater than 1
connections_count
.get(n)
.cloned()
.map_or(false, |count| count > 1)
} else if let ControlFlowGraphNode::StructField { .. } = &self.graph[*n] {
// Consider struct field alive when count is greater than 0
connections_count
.get(n)
.cloned()
.map_or(false, |count| count > 0)
} else {
false
match &self.graph[*n] {
ControlFlowGraphNode::ProgramNode {
node:
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::VariableDecl { .. }),
..
},
..
} => {
// Consider variables declarations alive when count is greater than 1
connections_count
.get(n)
.cloned()
.map_or(false, |count| count > 1)
}
ControlFlowGraphNode::ProgramNode {
node:
ty::TyAstNode {
content: ty::TyAstNodeContent::Declaration(ty::TyDecl::ImplTrait { .. }),
..
},
..
} => {
// Consider impls always alive.
// Consider it alive when it does not have any methods.
// Also consider it alive when it contains unused methods inside.
true
}
ControlFlowGraphNode::StructField { .. } => {
// Consider struct field alive when count is greater than 0
connections_count
.get(n)
.cloned()
.map_or(false, |count| count > 0)
}
_ => false,
}
};

Expand Down Expand Up @@ -1892,14 +1907,10 @@ fn construct_dead_code_warning_from_node(
content: ty::TyAstNodeContent::Declaration(ty::TyDecl::ImplTrait { decl_id, .. }),
span,
} => {
let ty::TyImplTrait { items: methods, .. } = decl_engine.get_impl_trait(decl_id);
if methods.is_empty() {
return None;
} else {
CompileWarning {
span: span.clone(),
warning_content: Warning::DeadDeclaration,
}
let ty::TyImplTrait { .. } = decl_engine.get_impl_trait(decl_id);
CompileWarning {
span: span.clone(),
warning_content: Warning::DeadDeclaration,
}
}
ty::TyAstNode {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
category = "fail"

# Consume `impl Alias2` form an earlier warning
#check: $() impl Alias2 {

#check: $()impl Alias2 {
#nextln: $()fn foo() {}
#nextln: $()Duplicate definitions for the method "foo" for type "type Alias2 = MyStruct1".
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'impl_unused_fn'
source = 'member'
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "impl_unused_fn"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
script;

struct Data1 {
value: u64
}

impl Data1 {
fn get(self) -> u64 {
self.value
}


fn get2(self) -> u64 {
self.value
}

fn get3(self) -> u64 {
self.value
}
}

fn main() -> u64 {
let d = Data1 { value: 42 };
d.get3()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
category = "compile"

# check: $()fn get(self) -> u64 {
# nextln: $()
# nextln: $()
# nextln: $()
# nextln: $()This method is never called.

# check: $()fn get2(self) -> u64 {
# nextln: $()
# nextln: $()
# nextln: $()
# nextln: $()This method is never called.

expected_warnings = 2

0 comments on commit 6f5cca5

Please sign in to comment.