mwojcik
/
nac3
forked from M-Labs/nac3
1
0
Fork 0
Code Pull Requests Activity

improve error messages 

#112, #110, #108, #87

Reviewed-on: M-Labs/nac3#145
Co-authored-by: ychenfo <yc@m-labs.hk>
Co-committed-by: ychenfo <yc@m-labs.hk>
This commit is contained in:
ychenfo 2021-12-22 08:52:19 +08:00 committed by sb10q
parent 0ff995722c
commit 66320679be
5 changed files with 158 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nac3artiq/src/lib.rs
View File

@ -437,7 +437,7 @@ impl Nac3 {
let (name, def_id, ty) = composer let (name, def_id, ty) = composer
.register_top_level(stmt.clone(), Some(resolver.clone()), path.clone()) .register_top_level(stmt.clone(), Some(resolver.clone()), path.clone())
.unwrap(); .map_err(|e| exceptions::PyRuntimeError::new_err(format!("nac3 compilation failure: {}", e)))?;
let id = *name_to_pyid.get(&name).unwrap(); let id = *name_to_pyid.get(&name).unwrap();
id_to_def.insert(id, def_id); id_to_def.insert(id, def_id);
if let Some(ty) = ty { if let Some(ty) = ty {
@ -515,7 +515,9 @@ impl Nac3 {
); );
let signature = store.add_cty(signature); let signature = store.add_cty(signature);
composer.start_analysis(true).unwrap(); composer.start_analysis(true).map_err(|e| exceptions::PyRuntimeError::new_err(format!(
"nac3 compilation failure: {}", e
)))?;
let top_level = Arc::new(composer.make_top_level_context()); let top_level = Arc::new(composer.make_top_level_context());
let instance = { let instance = {
let defs = top_level.definitions.read(); let defs = top_level.definitions.read();

4
nac3core/src/symbol_resolver.rs
View File

@ -298,10 +298,10 @@ pub fn parse_type_annotation<T>(
if let Name { id, .. } = &value.node { if let Name { id, .. } = &value.node {
subscript_name_handle(id, slice, unifier) subscript_name_handle(id, slice, unifier)
} else { } else {
Err("unsupported type expression".into()) Err(format!("unsupported type expression at {}", expr.location))
} }
} }
_ => Err("unsupported type expression".into()), _ => Err(format!("unsupported type expression at {}", expr.location)),
} }
} }

146
nac3core/src/toplevel/composer.rs
View File

@ -162,14 +162,24 @@ impl TopLevelComposer {
match &ast.node { match &ast.node {
ast::StmtKind::ClassDef { name: class_name, body, .. } => { ast::StmtKind::ClassDef { name: class_name, body, .. } => {
if self.keyword_list.contains(class_name) { if self.keyword_list.contains(class_name) {
return Err("cannot use keyword as a class name".into()); return Err(format!(
"cannot use keyword `{}` as a class name ({} at {})",
class_name,
mod_path,
ast.location
));
} }
if !defined_names.insert({ if !defined_names.insert({
let mut n = mod_path.clone(); let mut n = mod_path.clone();
n.push_str(&class_name.to_string()); n.push_str(&class_name.to_string());
n n
}) { }) {
return Err("duplicate definition of class".into()); return Err(format!(
"duplicate definition of class `{}` ({} at {})",
class_name,
mod_path,
ast.location
));
} }
let class_name = *class_name; let class_name = *class_name;
@ -212,7 +222,12 @@ impl TopLevelComposer {
contains_constructor = true; contains_constructor = true;
} }
if self.keyword_list.contains(method_name) { if self.keyword_list.contains(method_name) {
return Err("cannot use keyword as a method name".into()); return Err(format!(
"cannot use keyword `{}` as a method name ({} at {})",
method_name,
mod_path,
b.location
));
} }
let global_class_method_name = { let global_class_method_name = {
let mut n = mod_path.clone(); let mut n = mod_path.clone();
@ -226,7 +241,12 @@ impl TopLevelComposer {
n n
}; };
if !defined_names.insert(global_class_method_name.clone()) { if !defined_names.insert(global_class_method_name.clone()) {
return Err("duplicate class method definition".into()); return Err(format!(
"class method `{}` defined twice ({} at {})",
&global_class_method_name[mod_path.len()..],
mod_path,
b.location
));
} }
let method_def_id = self.definition_ast_list.len() + { let method_def_id = self.definition_ast_list.len() + {
// plus 1 here since we already have the class def // plus 1 here since we already have the class def
@ -283,12 +303,17 @@ impl TopLevelComposer {
// return Err("cannot use keyword as a top level function name".into()); // return Err("cannot use keyword as a top level function name".into());
// } // }
let global_fun_name = { let global_fun_name = {
let mut n = mod_path; let mut n = mod_path.clone();
n.push_str(&name.to_string()); n.push_str(&name.to_string());
n n
}; };
if !defined_names.insert(global_fun_name.clone()) { if !defined_names.insert(global_fun_name.clone()) {
return Err("duplicate top level function define".into()); return Err(format!(
"top level function `{}` defined twice ({} at {})",
&global_fun_name[mod_path.len()..],
mod_path,
ast.location
));
} }
let fun_name = *name; let fun_name = *name;
@ -314,7 +339,11 @@ impl TopLevelComposer {
)) ))
} }
_ => Err("only registrations of top level classes/functions are supported".into()), _ => Err(format!(
"registrations of constructs other than top level classes/functions are not supported ({} at {})",
mod_path,
ast.location
)),
} }
} }
@ -376,7 +405,10 @@ impl TopLevelComposer {
if !is_generic { if !is_generic {
is_generic = true; is_generic = true;
} else { } else {
return Err("Only single Generic[...] can be in bases".into()); return Err(format!(
"only single Generic[...] is allowed (at {})",
b.location
));
} }
let type_var_list: Vec<&ast::Expr<()>>; let type_var_list: Vec<&ast::Expr<()>>;
@ -414,7 +446,10 @@ impl TopLevelComposer {
}) })
}; };
if !all_unique_type_var { if !all_unique_type_var {
return Err("expect unique type variables".into()); return Err(format!(
"duplicate type variable occurs (at {})",
slice.location
));
} }
// add to TopLevelDef // add to TopLevelDef
@ -479,9 +514,11 @@ impl TopLevelComposer {
} }
if has_base { if has_base {
return Err("a class def can only have at most one base class \ return Err(format!(
declaration and one generic declaration" "a class definition can only have at most one base class \
.into()); declaration and one generic declaration (at {})",
b.location
));
} }
has_base = true; has_base = true;
@ -499,7 +536,10 @@ impl TopLevelComposer {
if let TypeAnnotation::CustomClass { .. } = &base_ty { if let TypeAnnotation::CustomClass { .. } = &base_ty {
class_ancestors.push(base_ty); class_ancestors.push(base_ty);
} else { } else {
return Err("class base declaration can only be custom class".into()); return Err(format!(
"class base declaration can only be custom class (at {})",
b.location,
));
} }
} }
} }
@ -668,14 +708,14 @@ impl TopLevelComposer {
let arg_types = { let arg_types = {
// make sure no duplicate parameter // make sure no duplicate parameter
let mut defined_paramter_name: HashSet<_> = HashSet::new(); let mut defined_paramter_name: HashSet<_> = HashSet::new();
let have_unique_fuction_parameter_name = args.args.iter().all(|x| { for x in args.args.iter() {
defined_paramter_name.insert(x.node.arg) if !defined_paramter_name.insert(x.node.arg) || keyword_list.contains(&x.node.arg) {
&& !keyword_list.contains(&x.node.arg) return Err(format!(
}); "top level function must have unique parameter names \
if !have_unique_fuction_parameter_name { and names should not be the same as the keywords (at {})",
return Err("top level function must have unique parameter names \ x.location
and names thould not be the same as the keywords" ));
.into()); }
} }
let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> = args
@ -902,20 +942,22 @@ impl TopLevelComposer {
// check method parameters cannot have same name // check method parameters cannot have same name
let mut defined_paramter_name: HashSet<_> = HashSet::new(); let mut defined_paramter_name: HashSet<_> = HashSet::new();
let zelf: StrRef = "self".into(); let zelf: StrRef = "self".into();
let have_unique_fuction_parameter_name = args.args.iter().all(|x| { for x in args.args.iter() {
defined_paramter_name.insert(x.node.arg) if !defined_paramter_name.insert(x.node.arg)
&& (!keyword_list.contains(&x.node.arg) || x.node.arg == zelf) || (keyword_list.contains(&x.node.arg) && x.node.arg != zelf) {
}); return Err(format!(
if !have_unique_fuction_parameter_name { "top level function must have unique parameter names \
return Err("class method must have unique parameter names \ and names should not be the same as the keywords (at {})",
and names thould not be the same as the keywords" x.location
.into()); ))
}
} }
if name == &"__init__".into() && !defined_paramter_name.contains(&zelf) { if name == &"__init__".into() && !defined_paramter_name.contains(&zelf) {
return Err("__init__ function must have a `self` parameter".into()); return Err(format!("__init__ method must have a `self` parameter (at {})", b.location));
} }
if !defined_paramter_name.contains(&zelf) { if !defined_paramter_name.contains(&zelf) {
return Err("currently does not support static method".into()); return Err(format!("class method must have a `self` parameter (at {})", b.location));
} }
let mut result = Vec::new(); let mut result = Vec::new();
@ -981,7 +1023,7 @@ impl TopLevelComposer {
None => None, None => None,
Some(default) => { Some(default) => {
if name == "self".into() { if name == "self".into() {
return Err(format!("`self` parameter cannot take default value at {}", x.location)); return Err(format!("`self` parameter cannot take default value (at {})", x.location));
} }
Some({ Some({
let v = Self::parse_parameter_default_value(default, class_resolver)?; let v = Self::parse_parameter_default_value(default, class_resolver)?;
@ -1090,7 +1132,7 @@ impl TopLevelComposer {
}; };
class_fields_def.push((*attr, dummy_field_type, mutable)); class_fields_def.push((*attr, dummy_field_type, mutable));
let annotation = parse_ast_to_type_annotation_kinds( let parsed_annotation = parse_ast_to_type_annotation_kinds(
class_resolver, class_resolver,
temp_def_list, temp_def_list,
unifier, unifier,
@ -1100,30 +1142,42 @@ impl TopLevelComposer {
)?; )?;
// find type vars within this return type annotation // find type vars within this return type annotation
let type_vars_within = let type_vars_within =
get_type_var_contained_in_type_annotation(&annotation); get_type_var_contained_in_type_annotation(&parsed_annotation);
// handle the class type var and the method type var // handle the class type var and the method type var
for type_var_within in type_vars_within { for type_var_within in type_vars_within {
if let TypeAnnotation::TypeVar(t) = type_var_within { if let TypeAnnotation::TypeVar(t) = type_var_within {
if !class_type_vars_def.contains(&t) { if !class_type_vars_def.contains(&t) {
return Err("class fields can only use type \ return Err(format!(
vars declared as class generic type vars" "class fields can only use type \
.into()); vars declared as class generic type vars (at {})",
annotation.location
));
} }
} else { } else {
unreachable!("must be type var annotation"); unreachable!("must be type var annotation");
} }
} }
type_var_to_concrete_def.insert(dummy_field_type, annotation); type_var_to_concrete_def.insert(dummy_field_type, parsed_annotation);
} else { } else {
return Err("same class fields defined twice".into()); return Err(format!(
"same class fields `{}` defined twice (at {})",
attr,
target.location
));
} }
} else { } else {
return Err("unsupported statement type in class definition body".into()); return Err(format!(
"unsupported statement type in class definition body (at {})",
target.location
));
} }
} }
ast::StmtKind::Pass { .. } => {} ast::StmtKind::Pass { .. } => {}
ast::StmtKind::Expr { value: _, .. } => {} // typically a docstring; ignoring all expressions matches CPython behavior ast::StmtKind::Expr { value: _, .. } => {} // typically a docstring; ignoring all expressions matches CPython behavior
_ => return Err("unsupported statement type in class definition body".into()), _ => return Err(format!(
"unsupported statement in class definition body (at {})",
b.location
)),
} }
} }
Ok(()) Ok(())
@ -1186,7 +1240,10 @@ impl TopLevelComposer {
type_var_to_concrete_def, type_var_to_concrete_def,
); );
if !ok { if !ok {
return Err("method has same name as ancestors' method, but incompatible type".into()); return Err(format!(
"method {} has same name as ancestors' method, but incompatible type",
class_method_name
));
} }
// mark it as added // mark it as added
is_override.insert(*class_method_name); is_override.insert(*class_method_name);
@ -1308,9 +1365,10 @@ impl TopLevelComposer {
for (f, _, _) in fields { for (f, _, _) in fields {
if !all_inited.contains(f) { if !all_inited.contains(f) {
return Err(format!( return Err(format!(
"fields `{}` of class `{}` not fully initialized", "fields `{}` of class `{}` not fully initialized in the initializer (at {})",
f, f,
class_name class_name,
body[0].location,
)); ));
} }
} }

10
nac3core/src/toplevel/test.rs
View File

@ -333,7 +333,7 @@ fn test_simple_function_analyze(source: Vec<&str>, tys: Vec<&str>, names: Vec<&s
pass pass
"} "}
], ],
vec!["application of type vars to generic class is not currently supported"]; vec!["application of type vars to generic class is not currently supported (at line 4 column 24)"];
"err no type var in generic app" "err no type var in generic app"
)] )]
#[test_case( #[test_case(
@ -389,7 +389,7 @@ fn test_simple_function_analyze(source: Vec<&str>, tys: Vec<&str>, names: Vec<&s
def __init__(): def __init__():
pass pass
"}], "}],
vec!["__init__ function must have a `self` parameter"]; vec!["__init__ method must have a `self` parameter (at line 2 column 5)"];
"err no self_1" "err no self_1"
)] )]
#[test_case( #[test_case(
@ -411,7 +411,7 @@ fn test_simple_function_analyze(source: Vec<&str>, tys: Vec<&str>, names: Vec<&s
"} "}
], ],
vec!["a class def can only have at most one base class declaration and one generic declaration"]; vec!["a class definition can only have at most one base class declaration and one generic declaration (at line 1 column 24)"];
"err multiple inheritance" "err multiple inheritance"
)] )]
#[test_case( #[test_case(
@ -436,7 +436,7 @@ fn test_simple_function_analyze(source: Vec<&str>, tys: Vec<&str>, names: Vec<&s
pass pass
"} "}
], ],
vec!["method has same name as ancestors' method, but incompatible type"]; vec!["method fun has same name as ancestors' method, but incompatible type"];
"err_incompatible_inheritance_method" "err_incompatible_inheritance_method"
)] )]
#[test_case( #[test_case(
@ -479,7 +479,7 @@ fn test_simple_function_analyze(source: Vec<&str>, tys: Vec<&str>, names: Vec<&s
pass pass
"} "}
], ],
vec!["duplicate definition of class"]; vec!["duplicate definition of class `A` ( at line 1 column 1)"];
"class same name" "class same name"
)] )]
fn test_analyze(source: Vec<&str>, res: Vec<&str>) { fn test_analyze(source: Vec<&str>, res: Vec<&str>) {

77
nac3core/src/toplevel/type_annotation.rs
View File

@ -70,7 +70,10 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if let TopLevelDef::Class { type_vars, .. } = &*def_read { if let TopLevelDef::Class { type_vars, .. } = &*def_read {
type_vars.clone() type_vars.clone()
} else { } else {
return Err("function cannot be used as a type".into()); return Err(format!(
"function cannot be used as a type (at {})",
expr.location
));
} }
} else { } else {
locked.get(&obj_id).unwrap().clone() locked.get(&obj_id).unwrap().clone()
@ -79,8 +82,9 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
// check param number here // check param number here
if !type_vars.is_empty() { if !type_vars.is_empty() {
return Err(format!( return Err(format!(
"expect {} type variable parameter but got 0", "expect {} type variable parameter but got 0 (at {})",
type_vars.len() type_vars.len(),
expr.location,
)); ));
} }
Ok(TypeAnnotation::CustomClass { id: obj_id, params: vec![] }) Ok(TypeAnnotation::CustomClass { id: obj_id, params: vec![] })
@ -88,10 +92,13 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if let TypeEnum::TVar { .. } = unifier.get_ty(ty).as_ref() { if let TypeEnum::TVar { .. } = unifier.get_ty(ty).as_ref() {
Ok(TypeAnnotation::TypeVar(ty)) Ok(TypeAnnotation::TypeVar(ty))
} else { } else {
Err("not a type variable identifier".into()) Err(format!(
"not a type variable identifier at {}",
expr.location
))
} }
} else { } else {
Err("name cannot be parsed as a type annotation".into()) Err(format!("name cannot be parsed as a type annotation at {}", expr.location))
} }
}; };
@ -100,7 +107,7 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if vec!["virtual".into(), "Generic".into(), "list".into(), "tuple".into()] if vec!["virtual".into(), "Generic".into(), "list".into(), "tuple".into()]
.contains(id) .contains(id)
{ {
return Err("keywords cannot be class name".into()); return Err(format!("keywords cannot be class name (at {})", expr.location));
} }
let obj_id = resolver let obj_id = resolver
.get_identifier_def(*id) .get_identifier_def(*id)
@ -126,13 +133,14 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
}; };
if type_vars.len() != params_ast.len() { if type_vars.len() != params_ast.len() {
return Err(format!( return Err(format!(
"expect {} type parameters but got {}", "expect {} type parameters but got {} (at {})",
type_vars.len(), type_vars.len(),
params_ast.len() params_ast.len(),
params_ast[0].location,
)); ));
} }
let result = params_ast let result = params_ast
.into_iter() .iter()
.map(|x| { .map(|x| {
parse_ast_to_type_annotation_kinds( parse_ast_to_type_annotation_kinds(
resolver, resolver,
@ -154,9 +162,11 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if no_type_var { if no_type_var {
result result
} else { } else {
return Err("application of type vars to generic class \ return Err(format!(
is not currently supported" "application of type vars to generic class \
.into()); is not currently supported (at {})",
params_ast[0].location
));
} }
}; };
Ok(TypeAnnotation::CustomClass { id: obj_id, params: param_type_infos }) Ok(TypeAnnotation::CustomClass { id: obj_id, params: param_type_infos })
@ -206,24 +216,27 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
matches!(&value.node, ast::ExprKind::Name { id, .. } if id == &"tuple".into()) matches!(&value.node, ast::ExprKind::Name { id, .. } if id == &"tuple".into())
} => } =>
{ {
if let ast::ExprKind::Tuple { elts, .. } = &slice.node { let tup_elts = {
let type_annotations = elts if let ast::ExprKind::Tuple { elts, .. } = &slice.node {
.iter() elts.as_slice()
.map(|e| { } else {
parse_ast_to_type_annotation_kinds( std::slice::from_ref(slice.as_ref())
resolver, }
top_level_defs, };
unifier, let type_annotations = tup_elts
primitives, .iter()
e, .map(|e| {
locked.clone(), parse_ast_to_type_annotation_kinds(
) resolver,
}) top_level_defs,
.collect::<Result<Vec<_>, _>>()?; unifier,
Ok(TypeAnnotation::Tuple(type_annotations)) primitives,
} else { e,
Err("Expect multiple elements for tuple".into()) locked.clone(),
} )
})
.collect::<Result<Vec<_>, _>>()?;
Ok(TypeAnnotation::Tuple(type_annotations))
} }
// custom class // custom class
@ -231,11 +244,11 @@ pub fn parse_ast_to_type_annotation_kinds<T>(
if let ast::ExprKind::Name { id, .. } = &value.node { if let ast::ExprKind::Name { id, .. } = &value.node {
class_name_handle(id, slice, unifier, locked) class_name_handle(id, slice, unifier, locked)
} else { } else {
Err("unsupported expression type for class name".into()) Err(format!("unsupported expression type for class name at {}", value.location))
} }
} }
_ => Err("unsupported expression for type annotation".into()), _ => Err(format!("unsupported expression for type annotation at {}", expr.location)),
} }
} }