Skip to content

Commit b38d6a8

Browse files
asteriteasterite
committed
feat: detect unconstructed structs
1 parent c5697da commit b38d6a8

File tree

7 files changed

+83
-35
lines changed

7 files changed

+83
-35
lines changed

‎compiler/noirc_frontend/src/elaborator/expressions.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::{
1010
},
1111
hir::{
1212
comptime::{self, InterpreterError},
13+
def_map::ModuleId,
1314
resolution::errors::ResolverError,
1415
type_check::{generics::TraitGenerics, TypeCheckError},
1516
},
@@ -532,6 +533,8 @@ impl<'context> Elaborator<'context> {
532533
}
533534
};
534535

536+
self.mark_struct_as_constructed(r#type.clone(), &last_segment.ident);
537+
535538
let turbofish_span = last_segment.turbofish_span();
536539

537540
let struct_generics = self.resolve_struct_turbofish_generics(
@@ -561,6 +564,15 @@ impl<'context> Elaborator<'context> {
561564
(expr, Type::Struct(struct_type, generics))
562565
}
563566

567+
fn mark_struct_as_constructed(&mut self, struct_type: Shared<StructType>, name: &Ident) {
568+
let struct_module_id = struct_type.borrow().id.module_id();
569+
let module_data = self.get_module(struct_module_id);
570+
let parent_local_id = module_data.parent.expect("Expected struct module parent to exist");
571+
let parent_module_id =
572+
ModuleId { krate: struct_module_id.krate, local_id: parent_local_id };
573+
self.interner.usage_tracker.mark_as_used(parent_module_id, name);
574+
}
575+
564576
/// Resolve all the fields of a struct constructor expression.
565577
/// Ensures all fields are present, none are repeated, and all
566578
/// are part of the struct.

‎compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ impl DefCollector {
508508
let ident = ident.clone();
509509
let error = CompilationError::ResolverError(ResolverError::UnusedItem {
510510
ident,
511-
item_type: unused_item.item_type(),
511+
item: *unused_item,
512512
});
513513
(error, module.location.file)
514514
})

‎compiler/noirc_frontend/src/hir/resolution/errors.rs‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ pub use noirc_errors::Span;
22
use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic};
33
use thiserror::Error;
44

5-
use crate::{ast::Ident, hir::comptime::InterpreterError, parser::ParserError, Type};
5+
use crate::{
6+
ast::Ident, hir::comptime::InterpreterError, parser::ParserError, usage_tracker::UnusedItem,
7+
Type,
8+
};
69

710
use super::import::PathResolutionError;
811

@@ -20,8 +23,8 @@ pub enum ResolverError {
2023
DuplicateDefinition { name: String, first_span: Span, second_span: Span },
2124
#[error("Unused variable")]
2225
UnusedVariable { ident: Ident },
23-
#[error("Unused {item_type}")]
24-
UnusedItem { ident: Ident, item_type: &'static str },
26+
#[error("Unused {}", item.item_type())]
27+
UnusedItem { ident: Ident, item: UnusedItem },
2528
#[error("Could not find variable in this scope")]
2629
VariableNotDeclared { name: String, span: Span },
2730
#[error("path is not an identifier")]
@@ -166,14 +169,26 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
166169
diagnostic.unnecessary = true;
167170
diagnostic
168171
}
169-
ResolverError::UnusedItem { ident, item_type } => {
172+
ResolverError::UnusedItem { ident, item} => {
170173
let name = &ident.0.contents;
174+
let item_type = item.item_type();
171175

172-
let mut diagnostic = Diagnostic::simple_warning(
173-
format!("unused {item_type} {name}"),
174-
format!("unused {item_type}"),
175-
ident.span(),
176-
);
176+
let mut diagnostic =
177+
if let UnusedItem::Struct(..) = item {
178+
Diagnostic::simple_warning(
179+
format!("{item_type} `{name}` is never constructed"),
180+
format!("{item_type} is never constructed"),
181+
ident.span(),
182+
)
183+
184+
} else {
185+
Diagnostic::simple_warning(
186+
format!("unused {item_type} {name}"),
187+
format!("unused {item_type}"),
188+
ident.span(),
189+
)
190+
191+
};
177192
diagnostic.unnecessary = true;
178193
diagnostic
179194
}

‎compiler/noirc_frontend/src/hir/resolution/import.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ fn resolve_name_in_module(
289289
return Err(PathResolutionError::Unresolved(first_segment.clone()));
290290
}
291291

292-
usage_tracker.mark_as_used(current_mod_id, first_segment);
292+
usage_tracker.mark_as_referenced(current_mod_id, first_segment);
293293

294294
let mut warning: Option<PathResolutionError> = None;
295295
for (index, (last_segment, current_segment)) in
@@ -356,7 +356,7 @@ fn resolve_name_in_module(
356356
return Err(PathResolutionError::Unresolved(current_segment.clone()));
357357
}
358358

359-
usage_tracker.mark_as_used(current_mod_id, current_segment);
359+
usage_tracker.mark_as_referenced(current_mod_id, current_segment);
360360

361361
current_ns = found_ns;
362362
}

‎compiler/noirc_frontend/src/tests.rs‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3241,14 +3241,13 @@ fn errors_on_unused_private_import() {
32413241
let errors = get_program_errors(src);
32423242
assert_eq!(errors.len(), 1);
32433243

3244-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3245-
&errors[0].0
3244+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
32463245
else {
32473246
panic!("Expected an unused item error");
32483247
};
32493248

32503249
assert_eq!(ident.to_string(), "bar");
3251-
assert_eq!(*item_type, "import");
3250+
assert_eq!(item.item_type(), "import");
32523251
}
32533252

32543253
#[test]
@@ -3277,14 +3276,13 @@ fn errors_on_unused_pub_crate_import() {
32773276
let errors = get_program_errors(src);
32783277
assert_eq!(errors.len(), 1);
32793278

3280-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3281-
&errors[0].0
3279+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
32823280
else {
32833281
panic!("Expected an unused item error");
32843282
};
32853283

32863284
assert_eq!(ident.to_string(), "bar");
3287-
assert_eq!(*item_type, "import");
3285+
assert_eq!(item.item_type(), "import");
32883286
}
32893287

32903288
#[test]
@@ -3413,14 +3411,13 @@ fn errors_on_unused_function() {
34133411
let errors = get_program_errors(src);
34143412
assert_eq!(errors.len(), 1);
34153413

3416-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3417-
&errors[0].0
3414+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
34183415
else {
34193416
panic!("Expected an unused item error");
34203417
};
34213418

34223419
assert_eq!(ident.to_string(), "foo");
3423-
assert_eq!(*item_type, "function");
3420+
assert_eq!(item.item_type(), "function");
34243421
}
34253422

34263423
#[test]
@@ -3429,6 +3426,8 @@ fn errors_on_unused_struct() {
34293426
struct Foo {}
34303427
struct Bar {}
34313428
3429+
pub fn foo(_: Foo) {}
3430+
34323431
fn main() {
34333432
let _ = Bar {};
34343433
}
@@ -3437,14 +3436,13 @@ fn errors_on_unused_struct() {
34373436
let errors = get_program_errors(src);
34383437
assert_eq!(errors.len(), 1);
34393438

3440-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3441-
&errors[0].0
3439+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
34423440
else {
34433441
panic!("Expected an unused item error");
34443442
};
34453443

34463444
assert_eq!(ident.to_string(), "Foo");
3447-
assert_eq!(*item_type, "struct");
3445+
assert_eq!(item.item_type(), "struct");
34483446
}
34493447

34503448
#[test]
@@ -3465,14 +3463,13 @@ fn errors_on_unused_trait() {
34653463
let errors = get_program_errors(src);
34663464
assert_eq!(errors.len(), 1);
34673465

3468-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3469-
&errors[0].0
3466+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
34703467
else {
34713468
panic!("Expected an unused item error");
34723469
};
34733470

34743471
assert_eq!(ident.to_string(), "Foo");
3475-
assert_eq!(*item_type, "trait");
3472+
assert_eq!(item.item_type(), "trait");
34763473
}
34773474

34783475
#[test]
@@ -3489,14 +3486,13 @@ fn errors_on_unused_type_alias() {
34893486
let errors = get_program_errors(src);
34903487
assert_eq!(errors.len(), 1);
34913488

3492-
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
3493-
&errors[0].0
3489+
let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0
34943490
else {
34953491
panic!("Expected an unused item error");
34963492
};
34973493

34983494
assert_eq!(ident.to_string(), "Foo");
3499-
assert_eq!(*item_type, "type alias");
3495+
assert_eq!(item.item_type(), "type alias");
35003496
}
35013497

35023498
#[test]

‎compiler/noirc_frontend/src/usage_tracker.rs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
node_interner::{FuncId, TraitId, TypeAliasId},
88
};
99

10-
#[derive(Debug)]
10+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1111
pub enum UnusedItem {
1212
Import,
1313
Function(FuncId),
@@ -51,8 +51,29 @@ impl UsageTracker {
5151
}
5252
}
5353

54+
/// Marks an item as being referenced. This doesn't always makes the item as used. For example
55+
/// if a struct is referenced it will still be considered unused unless it's constructed somewhere.
56+
pub(crate) fn mark_as_referenced(&mut self, current_mod_id: ModuleId, name: &Ident) {
57+
let Some(items) = self.unused_items.get_mut(&current_mod_id) else {
58+
return;
59+
};
60+
61+
let Some(unused_item) = items.get(name) else {
62+
return;
63+
};
64+
65+
if let UnusedItem::Struct(_) = unused_item {
66+
return;
67+
}
68+
69+
items.remove(name);
70+
}
71+
72+
/// Marks an item as being used.
5473
pub(crate) fn mark_as_used(&mut self, current_mod_id: ModuleId, name: &Ident) {
55-
self.unused_items.entry(current_mod_id).or_default().remove(name);
74+
if let Some(items) = self.unused_items.get_mut(&current_mod_id) {
75+
items.remove(name);
76+
};
5677
}
5778

5879
pub(crate) fn unused_items(&self) -> &HashMap<ModuleId, HashMap<Ident, UnusedItem>> {

‎noir_stdlib/src/meta/mod.nr‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,15 @@ mod tests {
238238
}
239239
// docs:end:big-derive-usage-example
240240

241+
impl DoNothing for Bar {
242+
fn do_nothing(_: Self) {}
243+
}
244+
241245
// This function is just to remove unused warnings
242246
fn remove_unused_warnings() {
243-
let _: Bar = crate::panic::panic(f"");
244-
let _: MyStruct = crate::panic::panic(f"");
245-
let _: MyOtherStruct = crate::panic::panic(f"");
247+
let _: Bar = Bar { x: 1, y: [2, 3] };
248+
let _: MyStruct = MyStruct { my_field: 1 };
249+
let _: MyOtherStruct = MyOtherStruct { my_other_field: 2 };
246250
let _ = derive_do_nothing(crate::panic::panic(f""));
247251
let _ = derive_do_nothing_alt(crate::panic::panic(f""));
248252
remove_unused_warnings();

0 commit comments

Comments
 (0)