Skip to content

Commit 3239a4a

Browse files
asteriteasterite
authored
chore: address some frontend tests TODOs (#7554)
1 parent ca21820 commit 3239a4a

File tree

15 files changed

+234
-119
lines changed

15 files changed

+234
-119
lines changed

‎compiler/noirc_frontend/src/ast/expression.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,13 @@ impl FunctionReturnType {
930930
FunctionReturnType::Ty(typ) => Cow::Borrowed(typ),
931931
}
932932
}
933+
934+
pub fn location(&self) -> Location {
935+
match self {
936+
FunctionReturnType::Default(location) => *location,
937+
FunctionReturnType::Ty(typ) => typ.location,
938+
}
939+
}
933940
}
934941

935942
impl Display for FunctionReturnType {

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,9 +1264,13 @@ impl<'context> Elaborator<'context> {
12641264
self.check_parent_traits_are_implemented(&trait_impl);
12651265
self.remove_trait_impl_assumed_trait_implementations(trait_impl.impl_id);
12661266

1267-
for (module, function, _) in &trait_impl.methods.functions {
1267+
for (module, function, noir_function) in &trait_impl.methods.functions {
12681268
self.local_module = *module;
1269-
let errors = check_trait_impl_method_matches_declaration(self.interner, *function);
1269+
let errors = check_trait_impl_method_matches_declaration(
1270+
self.interner,
1271+
*function,
1272+
noir_function,
1273+
);
12701274
self.push_errors(errors.into_iter().map(|error| error.into()));
12711275
}
12721276

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,19 @@ impl Elaborator<'_> {
9191
let type_contains_unspecified = let_stmt.r#type.contains_unspecified();
9292
let annotated_type = self.resolve_inferred_type(let_stmt.r#type);
9393

94+
let pattern_location = let_stmt.pattern.location();
9495
let expr_location = let_stmt.expression.location;
9596
let (expression, expr_type) =
9697
self.elaborate_expression_with_target_type(let_stmt.expression, Some(&annotated_type));
9798

9899
// Require the top-level of a global's type to be fully-specified
99100
if type_contains_unspecified && global_id.is_some() {
100101
let expected_type = annotated_type.clone();
101-
let error =
102-
ResolverError::UnspecifiedGlobalType { location: expr_location, expected_type };
102+
let error = ResolverError::UnspecifiedGlobalType {
103+
pattern_location,
104+
expr_location,
105+
expected_type,
106+
};
103107
self.push_err(error);
104108
}
105109

@@ -202,19 +206,18 @@ impl Elaborator<'_> {
202206
);
203207

204208
// Check that start range and end range have the same types
205-
let range_location = start_location.merge(end_location);
206209
self.unify(&start_range_type, &end_range_type, || TypeCheckError::TypeMismatch {
207210
expected_typ: start_range_type.to_string(),
208211
expr_typ: end_range_type.to_string(),
209-
expr_location: range_location,
212+
expr_location: end_location,
210213
});
211214

212215
let expected_type = self.polymorphic_integer();
213216

214217
self.unify(&start_range_type, &expected_type, || TypeCheckError::TypeCannotBeUsed {
215218
typ: start_range_type.clone(),
216219
place: "for loop",
217-
location: range_location,
220+
location: start_location,
218221
});
219222

220223
self.interner.push_definition_type(identifier.id, start_range_type);

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ impl Elaborator<'_> {
273273
pub(crate) fn check_trait_impl_method_matches_declaration(
274274
interner: &mut NodeInterner,
275275
function: FuncId,
276+
noir_function: &NoirFunction,
276277
) -> Vec<TypeCheckError> {
277278
let meta = interner.function_meta(&function);
278279
let modifiers = interner.function_modifiers(&function);
@@ -349,6 +350,8 @@ pub(crate) fn check_trait_impl_method_matches_declaration(
349350
definition_type,
350351
method_name,
351352
&meta.parameters,
353+
&meta.return_type,
354+
noir_function,
352355
meta.name.location,
353356
&trait_info.name.0.contents,
354357
&mut errors,
@@ -358,11 +361,14 @@ pub(crate) fn check_trait_impl_method_matches_declaration(
358361
errors
359362
}
360363

364+
#[allow(clippy::too_many_arguments)]
361365
fn check_function_type_matches_expected_type(
362366
expected: &Type,
363367
actual: &Type,
364368
method_name: &str,
365369
actual_parameters: &Parameters,
370+
actual_return_type: &FunctionReturnType,
371+
noir_function: &NoirFunction,
366372
location: Location,
367373
trait_name: &str,
368374
errors: &mut Vec<TypeCheckError>,
@@ -381,11 +387,16 @@ fn check_function_type_matches_expected_type(
381387
if params_a.len() == params_b.len() {
382388
for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() {
383389
if a.try_unify(b, &mut bindings).is_err() {
390+
let parameter_location = noir_function.def.parameters.get(i);
391+
let parameter_location = parameter_location.map(|param| param.typ.location);
392+
let parameter_location =
393+
parameter_location.unwrap_or_else(|| actual_parameters.0[i].0.location());
394+
384395
errors.push(TypeCheckError::TraitMethodParameterTypeMismatch {
385396
method_name: method_name.to_string(),
386397
expected_typ: a.to_string(),
387398
actual_typ: b.to_string(),
388-
parameter_location: actual_parameters.0[i].0.location(),
399+
parameter_location,
389400
parameter_index: i + 1,
390401
});
391402
}
@@ -395,7 +406,7 @@ fn check_function_type_matches_expected_type(
395406
errors.push(TypeCheckError::TypeMismatch {
396407
expected_typ: ret_a.to_string(),
397408
expr_typ: ret_b.to_string(),
398-
expr_location: location,
409+
expr_location: actual_return_type.location(),
399410
});
400411
}
401412
} else {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ impl Elaborator<'_> {
174174

175175
if !kind.unifies(&resolved_type.kind()) {
176176
let expected_typ_err = CompilationError::TypeError(TypeCheckError::TypeKindMismatch {
177-
expected_kind: kind.to_string(),
178-
expr_kind: resolved_type.kind().to_string(),
177+
expected_kind: kind.clone(),
178+
expr_kind: resolved_type.kind(),
179179
expr_location: location,
180180
});
181181
self.push_err(expected_typ_err);
@@ -523,8 +523,8 @@ impl Elaborator<'_> {
523523
(Type::Constant(lhs, lhs_kind), Type::Constant(rhs, rhs_kind)) => {
524524
if !lhs_kind.unifies(&rhs_kind) {
525525
self.push_err(TypeCheckError::TypeKindMismatch {
526-
expected_kind: lhs_kind.to_string(),
527-
expr_kind: rhs_kind.to_string(),
526+
expected_kind: lhs_kind,
527+
expr_kind: rhs_kind,
528528
expr_location: location,
529529
});
530530
return Type::Error;
@@ -557,8 +557,8 @@ impl Elaborator<'_> {
557557
fn check_kind(&mut self, typ: Type, expected_kind: &Kind, location: Location) -> Type {
558558
if !typ.kind().unifies(expected_kind) {
559559
self.push_err(TypeCheckError::TypeKindMismatch {
560-
expected_kind: expected_kind.to_string(),
561-
expr_kind: typ.kind().to_string(),
560+
expected_kind: expected_kind.clone(),
561+
expr_kind: typ.kind(),
562562
expr_location: location,
563563
});
564564
return Type::Error;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ pub enum DefCollectorErrorKind {
7777
impl DefCollectorErrorKind {
7878
pub fn location(&self) -> Location {
7979
match self {
80-
DefCollectorErrorKind::Duplicate { first_def: ident, .. }
80+
DefCollectorErrorKind::Duplicate { second_def: ident, .. }
8181
| DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: ident, .. }
8282
| DefCollectorErrorKind::CannotReexportItemWithLessVisibility {
8383
item_name: ident,
@@ -160,10 +160,10 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
160160
let second_location = second_def.0.location();
161161
let mut diag = Diagnostic::simple_error(
162162
primary_message,
163-
format!("First {} found here", &typ),
164-
first_location,
163+
format!("Second {} found here", &typ),
164+
second_location,
165165
);
166-
diag.add_secondary(format!("Second {} found here", &typ), second_location);
166+
diag.add_secondary(format!("First {} found here", &typ), first_location);
167167
diag
168168
}
169169
}

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,11 @@ pub enum ResolverError {
109109
#[error("Only `comptime` globals can be mutable")]
110110
MutableGlobal { location: Location },
111111
#[error("Globals must have a specified type")]
112-
UnspecifiedGlobalType { location: Location, expected_type: Type },
112+
UnspecifiedGlobalType {
113+
pattern_location: Location,
114+
expr_location: Location,
115+
expected_type: Type,
116+
},
113117
#[error("Global failed to evaluate")]
114118
UnevaluatedGlobalType { location: Location },
115119
#[error("Globals used in a type position must be non-negative")]
@@ -168,7 +172,7 @@ pub enum ResolverError {
168172
AttributeFunctionIsNotAPath { function: String, location: Location },
169173
#[error("Attribute function `{name}` is not in scope")]
170174
AttributeFunctionNotInScope { name: String, location: Location },
171-
#[error("The trait `{missing_trait}` is not implemented for `{type_missing_trait}")]
175+
#[error("The trait `{missing_trait}` is not implemented for `{type_missing_trait}`")]
172176
TraitNotImplemented {
173177
impl_trait: String,
174178
missing_trait: String,
@@ -197,7 +201,7 @@ pub enum ResolverError {
197201
impl ResolverError {
198202
pub fn location(&self) -> Location {
199203
match self {
200-
ResolverError::DuplicateDefinition { first_location: location, .. }
204+
ResolverError::DuplicateDefinition { second_location: location, .. }
201205
| ResolverError::UnconditionalRecursion { location, .. }
202206
| ResolverError::PathIsNotIdent { location }
203207
| ResolverError::Expected { location, .. }
@@ -233,7 +237,7 @@ impl ResolverError {
233237
| ResolverError::WhileInConstrainedFn { location }
234238
| ResolverError::JumpOutsideLoop { location, .. }
235239
| ResolverError::MutableGlobal { location }
236-
| ResolverError::UnspecifiedGlobalType { location, .. }
240+
| ResolverError::UnspecifiedGlobalType { pattern_location: location, .. }
237241
| ResolverError::UnevaluatedGlobalType { location }
238242
| ResolverError::NegativeGlobalType { location, .. }
239243
| ResolverError::NonIntegralGlobalType { location, .. }
@@ -292,10 +296,10 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
292296
ResolverError::DuplicateDefinition { name, first_location, second_location} => {
293297
let mut diag = Diagnostic::simple_error(
294298
format!("duplicate definitions of {name} found"),
295-
"first definition found here".to_string(),
296-
*first_location,
299+
"second definition found here".to_string(),
300+
*second_location,
297301
);
298-
diag.add_secondary("second definition found here".to_string(), *second_location);
302+
diag.add_secondary("first definition found here".to_string(), *first_location);
299303
diag
300304
}
301305
ResolverError::UnusedVariable { ident } => {
@@ -573,12 +577,14 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
573577
*location,
574578
)
575579
},
576-
ResolverError::UnspecifiedGlobalType { location, expected_type } => {
577-
Diagnostic::simple_error(
580+
ResolverError::UnspecifiedGlobalType { pattern_location, expr_location, expected_type } => {
581+
let mut diagnostic = Diagnostic::simple_error(
578582
"Globals must have a specified type".to_string(),
579-
format!("Inferred type is `{expected_type}`"),
580-
*location,
581-
)
583+
String::new(),
584+
*pattern_location,
585+
);
586+
diagnostic.add_secondary(format!("Inferred type is `{expected_type}`"), *expr_location);
587+
diagnostic
582588
},
583589
ResolverError::UnevaluatedGlobalType { location } => {
584590
Diagnostic::simple_error(
@@ -763,9 +769,9 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
763769
ResolverError::TraitNotImplemented { impl_trait, missing_trait: the_trait, type_missing_trait: typ, location, missing_trait_location} => {
764770
let mut diagnostic = Diagnostic::simple_error(
765771
format!("The trait bound `{typ}: {the_trait}` is not satisfied"),
766-
format!("The trait `{the_trait}` is not implemented for `{typ}")
772+
format!("The trait `{the_trait}` is not implemented for `{typ}`")
767773
, *location);
768-
diagnostic.add_secondary(format!("required by this bound in `{impl_trait}"), *missing_trait_location);
774+
diagnostic.add_secondary(format!("required by this bound in `{impl_trait}`"), *missing_trait_location);
769775
diagnostic
770776
},
771777
ResolverError::LoopNotYetSupported { location } => {

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

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub enum TypeCheckError {
6363
#[error("Expected type {expected} is not the same as {actual}")]
6464
TypeMismatchWithSource { expected: Type, actual: Type, location: Location, source: Source },
6565
#[error("Expected type {expected_kind:?} is not the same as {expr_kind:?}")]
66-
TypeKindMismatch { expected_kind: String, expr_kind: String, expr_location: Location },
66+
TypeKindMismatch { expected_kind: Kind, expr_kind: Kind, expr_location: Location },
6767
#[error("Evaluating {to} resulted in {to_value}, but {from_value} was expected")]
6868
TypeCanonicalizationMismatch {
6969
to: Type,
@@ -369,11 +369,37 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic {
369369
)
370370
}
371371
TypeCheckError::TypeKindMismatch { expected_kind, expr_kind, expr_location } => {
372-
Diagnostic::simple_error(
373-
format!("Expected kind {expected_kind}, found kind {expr_kind}"),
374-
String::new(),
375-
*expr_location,
376-
)
372+
// Try to improve the error message for some kind combinations
373+
match (expected_kind, expr_kind) {
374+
(Kind::Normal, Kind::Numeric(_)) => {
375+
Diagnostic::simple_error(
376+
"Expected type, found numeric generic".into(),
377+
"not a type".into(),
378+
*expr_location,
379+
)
380+
}
381+
(Kind::Numeric(typ), Kind::Normal) => {
382+
Diagnostic::simple_error(
383+
"Type provided when a numeric generic was expected".into(),
384+
format!("the numeric generic is not of type `{typ}`"),
385+
*expr_location,
386+
)
387+
}
388+
(Kind::Numeric(expected_type), Kind::Numeric(found_type)) => {
389+
Diagnostic::simple_error(
390+
format!("The numeric generic is not of type `{expected_type}`"),
391+
format!("expected `{expected_type}`, found `{found_type}`"),
392+
*expr_location,
393+
)
394+
}
395+
_ => {
396+
Diagnostic::simple_error(
397+
format!("Expected kind {expected_kind}, found kind {expr_kind}"),
398+
String::new(),
399+
*expr_location,
400+
)
401+
}
402+
}
377403
}
378404
TypeCheckError::TypeCanonicalizationMismatch { to, from, to_value, from_value, location } => {
379405
Diagnostic::simple_error(

‎compiler/noirc_frontend/src/hir_def/types.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -858,8 +858,8 @@ impl TypeVariable {
858858
) -> Result<(), TypeCheckError> {
859859
if !binding.kind().unifies(kind) {
860860
return Err(TypeCheckError::TypeKindMismatch {
861-
expected_kind: format!("{}", kind),
862-
expr_kind: format!("{}", binding.kind()),
861+
expected_kind: kind.clone(),
862+
expr_kind: binding.kind(),
863863
expr_location: location,
864864
});
865865
}
@@ -2144,8 +2144,8 @@ impl Type {
21442144
kind.ensure_value_fits(x, location)
21452145
} else {
21462146
Err(TypeCheckError::TypeKindMismatch {
2147-
expected_kind: format!("{}", constant_kind),
2148-
expr_kind: format!("{}", kind),
2147+
expected_kind: constant_kind,
2148+
expr_kind: kind.clone(),
21492149
expr_location: location,
21502150
})
21512151
}
@@ -2166,8 +2166,8 @@ impl Type {
21662166
op.function(lhs_value, rhs_value, &infix_kind, location)
21672167
} else {
21682168
Err(TypeCheckError::TypeKindMismatch {
2169-
expected_kind: format!("{}", kind),
2170-
expr_kind: format!("{}", infix_kind),
2169+
expected_kind: kind.clone(),
2170+
expr_kind: infix_kind,
21712171
expr_location: location,
21722172
})
21732173
}

‎compiler/noirc_frontend/src/node_interner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ impl NodeInterner {
765765
id: type_id,
766766
name: unresolved_trait.trait_def.name.clone(),
767767
crate_id: unresolved_trait.crate_id,
768-
location: unresolved_trait.trait_def.location,
768+
location: unresolved_trait.trait_def.name.location(),
769769
generics,
770770
visibility: ItemVisibility::Private,
771771
self_type_typevar: TypeVariable::unbound(self.next_type_variable_id(), Kind::Normal),

0 commit comments

Comments
 (0)