Skip to content

Commit

Permalink
fix: don't warn on unused struct that has an abi annotation (#6254)
Browse files Browse the repository at this point in the history
# Description

## Problem

Part of #6250

## Summary

Also renames a method to have a clearer name.

## Additional Context

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Oct 9, 2024
1 parent 2a727b3 commit 8a31632
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 17 deletions.
6 changes: 6 additions & 0 deletions compiler/noirc_frontend/src/ast/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ pub struct NoirStruct {
pub span: Span,
}

impl NoirStruct {
pub fn is_abi(&self) -> bool {
self.attributes.iter().any(|attr| attr.is_abi())
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct StructField {
pub visibility: ItemVisibility,
Expand Down
29 changes: 18 additions & 11 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ impl<'context> Elaborator<'context> {
);

if is_entry_point {
self.mark_parameter_type_as_used(&typ);
self.mark_type_as_used(&typ);
}

let pattern = self.elaborate_pattern_and_store_ids(
Expand Down Expand Up @@ -832,33 +832,33 @@ impl<'context> Elaborator<'context> {
self.current_item = None;
}

fn mark_parameter_type_as_used(&mut self, typ: &Type) {
fn mark_type_as_used(&mut self, typ: &Type) {
match typ {
Type::Array(_n, typ) => self.mark_parameter_type_as_used(typ),
Type::Slice(typ) => self.mark_parameter_type_as_used(typ),
Type::Array(_n, typ) => self.mark_type_as_used(typ),
Type::Slice(typ) => self.mark_type_as_used(typ),
Type::Tuple(types) => {
for typ in types {
self.mark_parameter_type_as_used(typ);
self.mark_type_as_used(typ);
}
}
Type::Struct(struct_type, generics) => {
self.mark_struct_as_constructed(struct_type.clone());
for generic in generics {
self.mark_parameter_type_as_used(generic);
self.mark_type_as_used(generic);
}
for (_, typ) in struct_type.borrow().get_fields(generics) {
self.mark_parameter_type_as_used(&typ);
self.mark_type_as_used(&typ);
}
}
Type::Alias(alias_type, generics) => {
self.mark_parameter_type_as_used(&alias_type.borrow().get_type(generics));
self.mark_type_as_used(&alias_type.borrow().get_type(generics));
}
Type::MutableReference(typ) => {
self.mark_parameter_type_as_used(typ);
self.mark_type_as_used(typ);
}
Type::InfixExpr(left, _op, right) => {
self.mark_parameter_type_as_used(left);
self.mark_parameter_type_as_used(right);
self.mark_type_as_used(left);
self.mark_type_as_used(right);
}
Type::FieldElement
| Type::Integer(..)
Expand Down Expand Up @@ -1277,6 +1277,13 @@ impl<'context> Elaborator<'context> {
self.local_module = typ.module_id;

let fields = self.resolve_struct_fields(&typ.struct_def, *type_id);

if typ.struct_def.is_abi() {
for field in &fields {
self.mark_type_as_used(&field.typ);
}
}

let fields_len = fields.len();
self.interner.update_struct(*type_id, |struct_def| {
struct_def.set_fields(fields);
Expand Down
14 changes: 8 additions & 6 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,12 +1013,14 @@ pub fn collect_struct(

let parent_module_id = ModuleId { krate, local_id: module_id };

interner.usage_tracker.add_unused_item(
parent_module_id,
name.clone(),
UnusedItem::Struct(id),
visibility,
);
if !unresolved.struct_def.is_abi() {
interner.usage_tracker.add_unused_item(
parent_module_id,
name.clone(),
UnusedItem::Struct(id),
visibility,
);
}

if let Err((first_def, second_def)) = result {
let error = DefCollectorErrorKind::Duplicate {
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,10 @@ impl SecondaryAttribute {
_ => false,
}
}

pub(crate) fn is_abi(&self) -> bool {
matches!(self, SecondaryAttribute::Abi(_))
}
}

impl fmt::Display for SecondaryAttribute {
Expand Down
30 changes: 30 additions & 0 deletions compiler/noirc_frontend/src/tests/unused_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,33 @@ fn no_warning_on_inner_struct_when_parent_is_used() {
let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}

#[test]
fn no_warning_on_struct_if_it_has_an_abi_attribute() {
let src = r#"
#[abi(functions)]
struct Foo {
a: Field,
}
fn main() {}
"#;
assert_no_errors(src);
}

#[test]
fn no_warning_on_indirect_struct_if_it_has_an_abi_attribute() {
let src = r#"
struct Bar {
field: Field,
}
#[abi(functions)]
struct Foo {
bar: Bar,
}
fn main() {}
"#;
assert_no_errors(src);
}

0 comments on commit 8a31632

Please sign in to comment.