Skip to content

Commit

Permalink
Add spans to compiler errors to prevent their incorrect deduplication (
Browse files Browse the repository at this point in the history
…#3838)

close #3380

~This PR only includes spans for compiler errors but does not include
them for compiler warnings. I can add it after getting some feedback on
this PR.~ I did some experiments and it looks like warnings are not
deduplicated in the same way as errors.

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
  • Loading branch information
anton-trunov and mohammadfawaz authored Jan 23, 2023
1 parent 86c02c2 commit 63670c4
Show file tree
Hide file tree
Showing 19 changed files with 126 additions and 31 deletions.
2 changes: 2 additions & 0 deletions sway-core/src/language/ty/declaration/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl TyStorageDeclaration {
None => {
errors.push(CompileError::StorageFieldDoesNotExist {
name: first_field.clone(),
span: first_field.span(),
});
return err(warnings, errors);
}
Expand Down Expand Up @@ -109,6 +110,7 @@ impl TyStorageDeclaration {
field_name: field.clone(),
available_fields: available_fields.join(", "),
struct_name: type_checked_buf.last().unwrap().name.clone(),
span: field.span(),
});
return err(warnings, errors);
}
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/language/ty/declaration/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ impl TyStructDeclaration {
.join("\n"),
field_name: field_to_access.clone(),
struct_name: self.name.clone(),
span: field_to_access.span(),
});
err(warnings, errors)
}
Expand Down
9 changes: 7 additions & 2 deletions sway-core/src/language/ty/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ impl TyProgram {
}

if !fn_declarations.insert(func.name.clone()) {
errors
.push(CompileError::MultipleDefinitionsOfFunction { name: func.name });
errors.push(CompileError::MultipleDefinitionsOfFunction {
name: func.name.clone(),
span: func.name.span(),
});
}

declarations.push(TyDeclaration::FunctionDeclaration(decl_id.clone()));
Expand Down Expand Up @@ -219,6 +221,7 @@ impl TyProgram {
if mains.len() > 1 {
errors.push(CompileError::MultipleDefinitionsOfFunction {
name: mains.last().unwrap().name.clone(),
span: mains.last().unwrap().name.span(),
});
}
let main_func = mains.remove(0);
Expand All @@ -241,6 +244,7 @@ impl TyProgram {
if mains.len() > 1 {
errors.push(CompileError::MultipleDefinitionsOfFunction {
name: mains.last().unwrap().name.clone(),
span: mains.last().unwrap().name.span(),
});
}
// A script must not return a `raw_ptr` or any type aggregating a `raw_slice`.
Expand Down Expand Up @@ -286,6 +290,7 @@ impl TyProgram {
if param.is_reference && param.is_mutable {
errors.push(CompileError::RefMutableNotAllowedInMain {
param_name: param.name.clone(),
span: param.name.span(),
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/abi.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use sway_error::error::CompileError;
use sway_types::Spanned;

use crate::{
error::*,
Expand Down Expand Up @@ -45,6 +46,7 @@ impl ty::TyAbiDeclaration {
if param.is_reference || param.is_mutable {
errors.push(CompileError::RefMutableNotAllowedInContractAbi {
param_name: param.name.clone(),
span: param.name.span(),
})
}
}
Expand All @@ -64,6 +66,7 @@ impl ty::TyAbiDeclaration {
if param.is_reference || param.is_mutable {
errors.push(CompileError::RefMutableNotAllowedInContractAbi {
param_name: param.name.clone(),
span: param.name.span(),
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ impl ty::TyFunctionParameter {
if !is_from_method {
let mutability = ty::VariableMutability::new_from_ref_mut(is_reference, is_mutable);
if mutability == ty::VariableMutability::Mutable {
errors.push(CompileError::MutableParameterNotSupported { param_name: name });
errors.push(CompileError::MutableParameterNotSupported {
param_name: name.clone(),
span: name.span(),
});
return err(warnings, errors);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,7 @@ fn type_check_impl_method(
if impld_method_ids.contains_key(&impl_method.name.clone()) {
errors.push(CompileError::MultipleDefinitionsOfFunction {
name: impl_method.name.clone(),
span: impl_method.name.span(),
});
return err(warnings, errors);
}
Expand Down Expand Up @@ -827,6 +828,7 @@ fn type_check_impl_method(
if impl_method_signature_param.is_mutable && !impl_method_signature_param.is_reference {
errors.push(CompileError::MutableParameterNotSupported {
param_name: impl_method_signature.name.clone(),
span: impl_method_signature.name.span(),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,12 +446,14 @@ impl ty::TyExpression {
errors.push(CompileError::NotAVariable {
name: name.clone(),
what_it_is: a.friendly_name(),
span,
});
ty::TyExpression::error(name.span(), engines)
}
None => {
errors.push(CompileError::UnknownVariable {
var_name: name.clone(),
span,
});
ty::TyExpression::error(name.span(), engines)
}
Expand Down Expand Up @@ -1189,7 +1191,8 @@ impl ty::TyExpression {
}
(false, None, None, None) => {
errors.push(CompileError::SymbolNotFound {
name: call_path_binding.inner.suffix,
name: call_path_binding.inner.suffix.clone(),
span: call_path_binding.inner.suffix.span(),
});
return err(warnings, errors);
}
Expand Down Expand Up @@ -1642,7 +1645,7 @@ impl ty::TyExpression {
errors
);
if !variable_decl.mutability.is_mutable() {
errors.push(CompileError::AssignmentToNonMutable { name });
errors.push(CompileError::AssignmentToNonMutable { name, span });
return err(warnings, errors);
}
break (name, variable_decl.body.return_type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ pub(crate) fn type_check_method_application(
None => {
errors.push(CompileError::StorageFieldDoesNotExist {
name: first_field.clone(),
span: first_field.span(),
});
return err(warnings, errors);
}
Expand Down
2 changes: 2 additions & 0 deletions sway-core/src/semantic_analysis/ast_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub(crate) fn reassign_storage_subfield(
None => {
errors.push(CompileError::StorageFieldDoesNotExist {
name: first_field.clone(),
span: first_field.span(),
});
return err(warnings, errors);
}
Expand Down Expand Up @@ -192,6 +193,7 @@ pub(crate) fn reassign_storage_subfield(
field_name: field.clone(),
available_fields: available_fields.join(", "),
struct_name: type_checked_buf.last().unwrap().name.clone(),
span: field.span(),
});
return err(warnings, errors);
}
Expand Down
7 changes: 6 additions & 1 deletion sway-core/src/semantic_analysis/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ impl Items {
pub(crate) fn check_symbol(&self, name: &Ident) -> Result<&ty::TyDeclaration, CompileError> {
self.symbols
.get(name)
.ok_or_else(|| CompileError::SymbolNotFound { name: name.clone() })
.ok_or_else(|| CompileError::SymbolNotFound {
name: name.clone(),
span: name.span(),
})
}

pub(crate) fn insert_trait_implementation_for_type(
Expand Down Expand Up @@ -216,6 +219,7 @@ impl Items {
None => {
errors.push(CompileError::UnknownVariable {
var_name: base_name.clone(),
span: base_name.span(),
});
return err(warnings, errors);
}
Expand Down Expand Up @@ -275,6 +279,7 @@ impl Items {
field_name: field_name.clone(),
struct_name,
available_fields: available_fields.join(", "),
span: field_name.span(),
});
return err(warnings, errors);
}
Expand Down
10 changes: 8 additions & 2 deletions sway-core/src/semantic_analysis/namespace/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,10 @@ impl Module {
errors
);
if visibility != Visibility::Public {
errors.push(CompileError::ImportPrivateSymbol { name: item.clone() });
errors.push(CompileError::ImportPrivateSymbol {
name: item.clone(),
span: item.span(),
});
}

let type_id = decl.return_type(engines, &item.span()).value;
Expand Down Expand Up @@ -394,7 +397,10 @@ impl Module {
};
}
None => {
errors.push(CompileError::SymbolNotFound { name: item.clone() });
errors.push(CompileError::SymbolNotFound {
name: item.clone(),
span: item.span(),
});
return err(warnings, errors);
}
};
Expand Down
3 changes: 2 additions & 1 deletion sway-core/src/type_system/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
Ident,
};
use sway_error::error::CompileError;
use sway_types::{integer_bits::IntegerBits, span::Span};
use sway_types::{integer_bits::IntegerBits, span::Span, Spanned};

use std::{
collections::HashSet,
Expand Down Expand Up @@ -1594,6 +1594,7 @@ impl TypeInfo {
field_name: first.clone(),
struct_name: name.clone(),
available_fields: available_fields.join(", "),
span: first.span(),
});
return err(warnings, errors);
}
Expand Down
46 changes: 24 additions & 22 deletions sway-error/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl fmt::Display for InterfaceName {
#[derive(Error, Debug, Clone, PartialEq, Eq, Hash)]
pub enum CompileError {
#[error("Variable \"{var_name}\" does not exist in this scope.")]
UnknownVariable { var_name: Ident },
UnknownVariable { var_name: Ident, span: Span },
#[error("Variable \"{var_name}\" does not exist in this scope.")]
UnknownVariablePath { var_name: Ident, span: Span },
#[error("Function \"{name}\" does not exist in this scope.")]
Expand All @@ -39,6 +39,7 @@ pub enum CompileError {
NotAVariable {
name: Ident,
what_it_is: &'static str,
span: Span,
},
#[error(
"Identifier \"{name}\" was called as if it was a function, but it is actually a \
Expand Down Expand Up @@ -104,7 +105,7 @@ pub enum CompileError {
#[error("Script declaration contains no main function. Scripts require a main function.")]
NoScriptMainFunction(Span),
#[error("Function \"{name}\" was already defined in scope.")]
MultipleDefinitionsOfFunction { name: Ident },
MultipleDefinitionsOfFunction { name: Ident, span: Span },
#[error(
"Attempted to reassign to a symbol that is not a variable. Symbol {name} is not a mutable \
variable, it is a {kind}."
Expand All @@ -115,7 +116,7 @@ pub enum CompileError {
span: Span,
},
#[error("Assignment to immutable variable. Variable {name} is not declared as mutable.")]
AssignmentToNonMutable { name: Ident },
AssignmentToNonMutable { name: Ident, span: Span },
#[error(
"Cannot call method \"{method_name}\" on variable \"{variable_name}\" because \
\"{variable_name}\" is not declared as mutable."
Expand All @@ -128,11 +129,11 @@ pub enum CompileError {
#[error(
"This parameter was declared as mutable, which is not supported yet, did you mean to use ref mut?"
)]
MutableParameterNotSupported { param_name: Ident },
MutableParameterNotSupported { param_name: Ident, span: Span },
#[error("Cannot pass immutable argument to mutable parameter.")]
ImmutableArgumentToMutableParameter { span: Span },
#[error("ref mut or mut parameter is not allowed for contract ABI function.")]
RefMutableNotAllowedInContractAbi { param_name: Ident },
RefMutableNotAllowedInContractAbi { param_name: Ident, span: Span },
#[error(
"Cannot call associated function \"{fn_name}\" as a method. Use associated function \
syntax instead."
Expand Down Expand Up @@ -296,11 +297,12 @@ pub enum CompileError {
field_name: Ident,
available_fields: String,
struct_name: Ident,
span: Span,
},
#[error("Could not find symbol \"{name}\" in this scope.")]
SymbolNotFound { name: Ident },
SymbolNotFound { name: Ident, span: Span },
#[error("Symbol \"{name}\" is private.")]
ImportPrivateSymbol { name: Ident },
ImportPrivateSymbol { name: Ident, span: Span },
#[error(
"Because this if expression's value is used, an \"else\" branch is required and it must \
return type \"{r#type}\""
Expand Down Expand Up @@ -621,15 +623,15 @@ pub enum CompileError {
#[error("Attempting to specify a contract method parameter for a non-contract function call")]
CallParamForNonContractCallMethod { span: Span },
#[error("Storage field {name} does not exist")]
StorageFieldDoesNotExist { name: Ident },
StorageFieldDoesNotExist { name: Ident, span: Span },
#[error("No storage has been declared")]
NoDeclaredStorage { span: Span },
#[error("Multiple storage declarations were found")]
MultipleStorageDeclarations { span: Span },
#[error("Type {ty} can only be declared directly as a storage field")]
InvalidStorageOnlyTypeDecl { ty: String, span: Span },
#[error("Expected identifier, found keyword \"{name}\" ")]
InvalidVariableName { name: Ident },
InvalidVariableName { name: Ident, span: Span },
#[error(
"Internal compiler error: Unexpected {decl_type} declaration found.\n\
Please file an issue on the repository and include the code that triggered this error."
Expand Down Expand Up @@ -681,7 +683,7 @@ pub enum CompileError {
#[error("The type \"{ty}\" is not allowed in storage.")]
TypeNotAllowedInContractStorage { ty: String, span: Span },
#[error("ref mut parameter not allowed for main()")]
RefMutableNotAllowedInMain { param_name: Ident },
RefMutableNotAllowedInMain { param_name: Ident, span: Span },
#[error("Returning a `raw_ptr` from `main()` is not allowed.")]
PointerReturnNotAllowedInMain { span: Span },
#[error(
Expand Down Expand Up @@ -733,10 +735,10 @@ impl Spanned for CompileError {
fn span(&self) -> Span {
use CompileError::*;
match self {
UnknownVariable { var_name } => var_name.span(),
UnknownVariable { span, .. } => span.clone(),
UnknownVariablePath { span, .. } => span.clone(),
UnknownFunction { span, .. } => span.clone(),
NotAVariable { name, .. } => name.span(),
NotAVariable { span, .. } => span.clone(),
NotAFunction { span, .. } => span.clone(),
Unimplemented(_, span) => span.clone(),
UnimplementedWithHelp(_, _, span) => span.clone(),
Expand All @@ -753,12 +755,12 @@ impl Spanned for CompileError {
NoPredicateMainFunction(span) => span.clone(),
PredicateMainDoesNotReturnBool(span) => span.clone(),
NoScriptMainFunction(span) => span.clone(),
MultipleDefinitionsOfFunction { name } => name.span(),
MultipleDefinitionsOfFunction { span, .. } => span.clone(),
ReassignmentToNonVariable { span, .. } => span.clone(),
AssignmentToNonMutable { name } => name.span(),
MutableParameterNotSupported { param_name } => param_name.span(),
AssignmentToNonMutable { span, .. } => span.clone(),
MutableParameterNotSupported { span, .. } => span.clone(),
ImmutableArgumentToMutableParameter { span } => span.clone(),
RefMutableNotAllowedInContractAbi { param_name } => param_name.span(),
RefMutableNotAllowedInContractAbi { span, .. } => span.clone(),
MethodRequiresMutableSelf { span, .. } => span.clone(),
AssociatedFunctionCalledAsMethod { span, .. } => span.clone(),
TypeParameterNotInTypeScope { span, .. } => span.clone(),
Expand All @@ -784,9 +786,9 @@ impl Spanned for CompileError {
NotAStruct { span, .. } => span.clone(),
NotIndexable { span, .. } => span.clone(),
FieldAccessOnNonStruct { span, .. } => span.clone(),
FieldNotFound { field_name, .. } => field_name.span(),
SymbolNotFound { name, .. } => name.span(),
ImportPrivateSymbol { name } => name.span(),
FieldNotFound { span, .. } => span.clone(),
SymbolNotFound { span, .. } => span.clone(),
ImportPrivateSymbol { span, .. } => span.clone(),
NoElseBranch { span, .. } => span.clone(),
UnqualifiedSelfType { span, .. } => span.clone(),
NotAType { span, .. } => span.clone(),
Expand Down Expand Up @@ -879,11 +881,11 @@ impl Spanned for CompileError {
ContractCallParamRepeated { span, .. } => span.clone(),
UnrecognizedContractParam { span, .. } => span.clone(),
CallParamForNonContractCallMethod { span, .. } => span.clone(),
StorageFieldDoesNotExist { name } => name.span(),
StorageFieldDoesNotExist { span, .. } => span.clone(),
InvalidStorageOnlyTypeDecl { span, .. } => span.clone(),
NoDeclaredStorage { span, .. } => span.clone(),
MultipleStorageDeclarations { span, .. } => span.clone(),
InvalidVariableName { name } => name.span(),
InvalidVariableName { span, .. } => span.clone(),
UnexpectedDeclaration { span, .. } => span.clone(),
ContractAddressMustBeKnown { span, .. } => span.clone(),
ConvertParseTree { error } => error.span(),
Expand All @@ -902,7 +904,7 @@ impl Spanned for CompileError {
ConfigTimeConstantNotAConstDecl { span } => span.clone(),
ConfigTimeConstantNotALiteral { span } => span.clone(),
TypeNotAllowedInContractStorage { span, .. } => span.clone(),
RefMutableNotAllowedInMain { param_name } => param_name.span(),
RefMutableNotAllowedInMain { span, .. } => span.clone(),
PointerReturnNotAllowedInMain { span } => span.clone(),
NestedSliceReturnNotAllowedInMain { span } => span.clone(),
InitializedRegisterReassignment { span, .. } => span.clone(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'error_deduplication'
source = 'member'
Loading

0 comments on commit 63670c4

Please sign in to comment.