Skip to content

Commit

Permalink
Improve spans in error messages around generic types (FuelLabs#1633)
Browse files Browse the repository at this point in the history

* Associate a Span with every Ref type
  • Loading branch information
sezna authored May 23, 2022
1 parent 142b359 commit 381a080
Show file tree
Hide file tree
Showing 18 changed files with 164 additions and 70 deletions.
2 changes: 1 addition & 1 deletion sway-core/src/optimize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2519,7 +2519,7 @@ fn convert_resolved_type(
span.clone(),
))
}
TypeInfo::Ref(_) => {
TypeInfo::Ref(..) => {
return Err(CompileError::Internal(
"Ref type cannot be resolved in IR.",
span.clone(),
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/parse_tree/declaration/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl EnumVariant {
let mut errors = vec![];
let enum_variant_type =
if let Some(matching_id) = self.r#type.matches_type_parameter(type_mapping) {
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, span))
} else {
check!(
namespace.resolve_type_with_self(self.r#type.clone(), self_type, span, false),
Expand Down
9 changes: 7 additions & 2 deletions sway-core/src/parse_tree/declaration/type_parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ impl From<&TypeParameter> for TypedDeclaration {
impl CopyTypes for TypeParameter {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.type_id = match look_up_type_id(self.type_id).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.type_id)),
Some(matching_id) => {
insert_type(TypeInfo::Ref(matching_id, self.name_ident.span().clone()))
}
None => {
let ty = TypeInfo::Ref(insert_type(look_up_type_id_raw(self.type_id)), self.span());
insert_type(ty)
}
};
}
}
Expand Down
28 changes: 20 additions & 8 deletions sway-core/src/semantic_analysis/ast_node/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,13 @@ impl CopyTypes for TypedTraitFn {
self.return_type = if let Some(matching_id) =
look_up_type_id(self.return_type).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, self.return_type_span.clone()))
} else {
insert_type(look_up_type_id_raw(self.return_type))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.return_type)),
self.return_type_span.clone(),
);
insert_type(ty)
};
}
}
Expand Down Expand Up @@ -516,16 +520,24 @@ pub struct TypedReassignment {
impl CopyTypes for TypedReassignment {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.rhs.copy_types(type_mapping);
self.lhs
.iter_mut()
.for_each(|ReassignmentLhs { ref mut r#type, .. }| {
self.lhs.iter_mut().for_each(
|ReassignmentLhs {
ref mut r#type,
name,
..
}| {
*r#type = if let Some(matching_id) =
look_up_type_id(*r#type).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, name.span().clone()))
} else {
insert_type(look_up_type_id_raw(*r#type))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(*r#type)),
name.span().clone(),
);
insert_type(ty)
};
});
},
);
}
}
8 changes: 6 additions & 2 deletions sway-core/src/semantic_analysis/ast_node/declaration/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,13 @@ impl CopyTypes for TypedEnumVariant {
self.r#type = if let Some(matching_id) =
look_up_type_id(self.r#type).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, self.span.clone()))
} else {
insert_type(look_up_type_id_raw(self.r#type))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.r#type)),
self.span.clone(),
);
insert_type(ty)
};
}
}
Expand Down
18 changes: 14 additions & 4 deletions sway-core/src/semantic_analysis/ast_node/declaration/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,16 @@ impl CopyTypes for TypedFunctionDeclaration {

self.return_type =
match look_up_type_id(self.return_type).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.return_type)),
Some(matching_id) => {
insert_type(TypeInfo::Ref(matching_id, self.return_type_span.clone()))
}
None => {
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.return_type)),
self.return_type_span.clone(),
);
insert_type(ty)
}
};

self.body.copy_types(type_mapping);
Expand Down Expand Up @@ -165,7 +173,9 @@ impl TypedFunctionDeclaration {
parameters.iter_mut().for_each(|parameter| {
parameter.type_id =
match look_up_type_id(parameter.type_id).matches_type_parameter(&type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
Some(matching_id) => {
insert_type(TypeInfo::Ref(matching_id, parameter.type_span.clone()))
}
None => check!(
fn_namespace.resolve_type_with_self(
look_up_type_id(parameter.type_id),
Expand Down Expand Up @@ -199,7 +209,7 @@ impl TypedFunctionDeclaration {
}

let return_type = match return_type.matches_type_parameter(&type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id, return_type_span.clone())),
None => check!(
fn_namespace.resolve_type_with_self(
return_type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ impl PartialEq for TypedFunctionParameter {
impl CopyTypes for TypedFunctionParameter {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.r#type = match look_up_type_id(self.r#type).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.r#type)),
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id, self.type_span.clone())),
None => {
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.r#type)),
self.type_span.clone(),
);
insert_type(ty)
}
};
}
}
10 changes: 8 additions & 2 deletions sway-core/src/semantic_analysis/ast_node/declaration/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,14 @@ impl PartialEq for TypedStructField {
impl CopyTypes for TypedStructField {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.r#type = match look_up_type_id(self.r#type).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.r#type)),
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id, self.span.clone())),
None => {
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.r#type)),
self.span.clone(),
);
insert_type(ty)
}
};
}
}
Expand Down
17 changes: 12 additions & 5 deletions sway-core/src/semantic_analysis/ast_node/declaration/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,18 @@ impl PartialEq for TypedVariableDeclaration {

impl CopyTypes for TypedVariableDeclaration {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.type_ascription =
match look_up_type_id(self.type_ascription).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.type_ascription)),
};
self.type_ascription = match look_up_type_id(self.type_ascription)
.matches_type_parameter(type_mapping)
{
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id, self.body.span.clone())),
None => {
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.type_ascription)),
self.body.span.clone(),
);
insert_type(ty)
}
};
self.body.copy_types(type_mapping)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,14 @@ impl CopyTypes for TypedExpression {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
self.return_type =
match look_up_type_id(self.return_type).matches_type_parameter(type_mapping) {
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id)),
None => insert_type(look_up_type_id_raw(self.return_type)),
Some(matching_id) => insert_type(TypeInfo::Ref(matching_id, self.span.clone())),
None => {
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(self.return_type)),
self.span.clone(),
);
insert_type(ty)
}
};

self.expression.copy_types(type_mapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,9 +411,13 @@ impl CopyTypes for TypedExpressionVariant {
*resolved_type_of_parent = if let Some(matching_id) =
look_up_type_id(*resolved_type_of_parent).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, field_to_access.span.clone()))
} else {
insert_type(look_up_type_id_raw(*resolved_type_of_parent))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(*resolved_type_of_parent)),
field_to_access.span.clone(),
);
insert_type(ty)
};

field_to_access.copy_types(type_mapping);
Expand All @@ -427,9 +431,13 @@ impl CopyTypes for TypedExpressionVariant {
*resolved_type_of_parent = if let Some(matching_id) =
look_up_type_id(*resolved_type_of_parent).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, prefix.span.clone()))
} else {
insert_type(look_up_type_id_raw(*resolved_type_of_parent))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(*resolved_type_of_parent)),
prefix.span.clone(),
);
insert_type(ty)
};

prefix.copy_types(type_mapping);
Expand All @@ -447,13 +455,15 @@ impl CopyTypes for TypedExpressionVariant {
AbiCast { address, .. } => address.copy_types(type_mapping),
// storage is never generic and cannot be monomorphized
StorageAccess { .. } => (),
TypeProperty { type_id, .. } => {
TypeProperty { type_id, span, .. } => {
*type_id = if let Some(matching_id) =
look_up_type_id(*type_id).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, span.clone()))
} else {
insert_type(look_up_type_id_raw(*type_id))
let ty =
TypeInfo::Ref(insert_type(look_up_type_id_raw(*type_id)), span.clone());
insert_type(ty)
};
}
SizeOfValue { expr } => expr.copy_types(type_mapping),
Expand All @@ -465,9 +475,13 @@ impl CopyTypes for TypedExpressionVariant {
*enum_type = if let Some(matching_id) =
look_up_type_id(*enum_type).matches_type_parameter(type_mapping)
{
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, variant.span.clone()))
} else {
insert_type(look_up_type_id_raw(*enum_type))
let ty = TypeInfo::Ref(
insert_type(look_up_type_id_raw(*enum_type)),
variant.span.clone(),
);
insert_type(ty)
};
variant.copy_types(type_mapping);
}
Expand Down
7 changes: 5 additions & 2 deletions sway-core/src/semantic_analysis/ast_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ impl TypedAstNode {
let r#type = match r#type.matches_type_parameter(&type_mapping)
{
Some(matching_id) => {
insert_type(TypeInfo::Ref(matching_id))
insert_type(TypeInfo::Ref(matching_id, type_span))
}
None => check!(
namespace.resolve_type_with_self(
Expand Down Expand Up @@ -930,7 +930,10 @@ fn reassignment(
errors.push(CompileError::AssignmentToNonMutable { name });
}
// the RHS is a ref type to the LHS
let rhs_type_id = insert_type(TypeInfo::Ref(variable_decl.body.return_type));
let rhs_type_id = insert_type(TypeInfo::Ref(
variable_decl.body.return_type,
variable_decl.body.span.clone(),
));
// type check the reassignment
let rhs = check!(
TypedExpression::type_check(TypeCheckArguments {
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl Root {
insert_type(TypeInfo::Array(elem_type_id, size))
}
TypeInfo::SelfType => self_type,
TypeInfo::Ref(id) => id,
TypeInfo::Ref(id, _sp) => id,
o => insert_type(o),
};
ok(type_id, warnings, errors)
Expand Down Expand Up @@ -288,7 +288,7 @@ impl Root {
);
insert_type(TypeInfo::Array(elem_type_id, size))
}
TypeInfo::Ref(id) => id,
TypeInfo::Ref(id, _sp) => id,
o => insert_type(o),
};
ok(type_id, warnings, errors)
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/node_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ fn type_info_name(type_info: &TypeInfo) -> String {
TypeInfo::Numeric => "numeric",
TypeInfo::Contract => "contract",
TypeInfo::ErrorRecovery => "err_recov",
TypeInfo::Ref(x) => return format!("T{}", x),
TypeInfo::Ref(x, _sp) => return format!("T{}", x),
TypeInfo::Unknown => "unknown",
TypeInfo::UnknownGeneric { name } => return format!("generic {}", name),
TypeInfo::ContractCaller { abi_name, .. } => {
Expand Down
8 changes: 4 additions & 4 deletions sway-core/src/type_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ fn chain_of_refs() {
let sp = Span::dummy();
// numerics
let id = engine.insert_type(TypeInfo::Numeric);
let id2 = engine.insert_type(TypeInfo::Ref(id));
let id3 = engine.insert_type(TypeInfo::Ref(id));
let id2 = engine.insert_type(TypeInfo::Ref(id, sp.clone()));
let id3 = engine.insert_type(TypeInfo::Ref(id, sp.clone()));
let id4 = engine.insert_type(TypeInfo::UnsignedInteger(IntegerBits::Eight));

// Unify them together...
Expand All @@ -182,8 +182,8 @@ fn chain_of_refs_2() {
let sp = Span::dummy();
// numerics
let id = engine.insert_type(TypeInfo::Numeric);
let id2 = engine.insert_type(TypeInfo::Ref(id));
let id3 = engine.insert_type(TypeInfo::Ref(id));
let id2 = engine.insert_type(TypeInfo::Ref(id, sp.clone()));
let id3 = engine.insert_type(TypeInfo::Ref(id, sp.clone()));
let id4 = engine.insert_type(TypeInfo::UnsignedInteger(IntegerBits::Eight));

// Unify them together...
Expand Down
Loading

0 comments on commit 381a080

Please sign in to comment.