Skip to content

Commit

Permalink
fix: Do not warn on unused functions marked with #[export] (#6625)
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh authored Nov 26, 2024
1 parent 7d7b9c9 commit 30f8378
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 19 deletions.
4 changes: 3 additions & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,7 @@ pub fn collect_function(
} else {
function.name() == MAIN_FUNCTION
};
let has_export = function.def.attributes.has_export();

let name = function.name_ident().clone();
let func_id = interner.push_empty_fn();
Expand All @@ -954,7 +955,7 @@ pub fn collect_function(
interner.register_function(func_id, &function.def);
}

if !is_test && !is_entry_point_function {
if !is_test && !is_entry_point_function && !has_export {
let item = UnusedItem::Function(func_id);
usage_tracker.add_unused_item(module, name.clone(), item, visibility);
}
Expand Down Expand Up @@ -1257,6 +1258,7 @@ pub(crate) fn collect_global(
// Add the statement to the scope so its path can be looked up later
let result = def_map.modules[module_id.0].declare_global(name.clone(), visibility, global_id);

// Globals marked as ABI don't have to be used.
if !is_abi {
let parent_module_id = ModuleId { krate: crate_id, local_id: module_id };
usage_tracker.add_unused_item(
Expand Down
6 changes: 1 addition & 5 deletions compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,7 @@ impl CrateDefMap {
module.value_definitions().filter_map(|id| {
if let Some(func_id) = id.as_function() {
let attributes = interner.function_attributes(&func_id);
if attributes.secondary.contains(&SecondaryAttribute::Export) {
Some(func_id)
} else {
None
}
attributes.has_export().then_some(func_id)
} else {
None
}
Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,9 +676,7 @@ impl Attributes {
/// This is useful for finding out if we should compile a contract method
/// as an entry point or not.
pub fn has_contract_library_method(&self) -> bool {
self.secondary
.iter()
.any(|attribute| attribute == &SecondaryAttribute::ContractLibraryMethod)
self.has_secondary_attr(&SecondaryAttribute::ContractLibraryMethod)
}

pub fn is_test_function(&self) -> bool {
Expand Down Expand Up @@ -718,11 +716,21 @@ impl Attributes {
}

pub fn has_varargs(&self) -> bool {
self.secondary.iter().any(|attr| matches!(attr, SecondaryAttribute::Varargs))
self.has_secondary_attr(&SecondaryAttribute::Varargs)
}

pub fn has_use_callers_scope(&self) -> bool {
self.secondary.iter().any(|attr| matches!(attr, SecondaryAttribute::UseCallersScope))
self.has_secondary_attr(&SecondaryAttribute::UseCallersScope)
}

/// True if the function is marked with an `#[export]` attribute.
pub fn has_export(&self) -> bool {
self.has_secondary_attr(&SecondaryAttribute::Export)
}

/// Check if secondary attributes contain a specific instance.
pub fn has_secondary_attr(&self, attr: &SecondaryAttribute) -> bool {
self.secondary.contains(attr)
}
}

Expand Down
38 changes: 30 additions & 8 deletions compiler/noirc_frontend/src/tests/unused_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,31 @@ fn does_not_warn_on_unused_global_if_it_has_an_abi_attribute() {
assert_no_errors(src);
}

#[test]
fn does_not_warn_on_unused_struct_if_it_has_an_abi_attribute() {
let src = r#"
#[abi(dummy)]
struct Foo { bar: u8 }
fn main() {}
"#;
assert_no_errors(src);
}

#[test]
fn does_not_warn_on_unused_function_if_it_has_an_export_attribute() {
let src = r#"
#[export]
fn foo() {}
fn main() {}
"#;
assert_no_errors(src);
}

#[test]
fn no_warning_on_inner_struct_when_parent_is_used() {
let src = r#"
let src = r#"
struct Bar {
inner: [Field; 3],
}
Expand All @@ -247,7 +269,7 @@ fn no_warning_on_inner_struct_when_parent_is_used() {

#[test]
fn no_warning_on_struct_if_it_has_an_abi_attribute() {
let src = r#"
let src = r#"
#[abi(functions)]
struct Foo {
a: Field,
Expand All @@ -260,7 +282,7 @@ fn no_warning_on_struct_if_it_has_an_abi_attribute() {

#[test]
fn no_warning_on_indirect_struct_if_it_has_an_abi_attribute() {
let src = r#"
let src = r#"
struct Bar {
field: Field,
}
Expand All @@ -277,7 +299,7 @@ fn no_warning_on_indirect_struct_if_it_has_an_abi_attribute() {

#[test]
fn no_warning_on_self_in_trait_impl() {
let src = r#"
let src = r#"
struct Bar {}
trait Foo {
Expand All @@ -298,18 +320,18 @@ fn no_warning_on_self_in_trait_impl() {
#[test]
fn resolves_trait_where_clause_in_the_correct_module() {
// This is a regression test for https://github.com/noir-lang/noir/issues/6479
let src = r#"
let src = r#"
mod foo {
pub trait Foo {}
}
use foo::Foo;
pub trait Bar<T>
where
T: Foo,
{}
fn main() {}
"#;
assert_no_errors(src);
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_frontend/src/usage_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub struct UsageTracker {
}

impl UsageTracker {
/// Register an item as unused, waiting to be marked as used later.
/// Things that should not emit warnings should not be added at all.
pub(crate) fn add_unused_item(
&mut self,
module_id: ModuleId,
Expand Down Expand Up @@ -73,6 +75,7 @@ impl UsageTracker {
};
}

/// Get all the unused items per module.
pub fn unused_items(&self) -> &HashMap<ModuleId, HashMap<Ident, UnusedItem>> {
&self.unused_items
}
Expand Down

0 comments on commit 30f8378

Please sign in to comment.