WIP: Adding static class attribute definition functionality #312

Draft
Aadityavardhan wants to merge 2 commits from Aadityavardhan/nac3_sca:master into master
4 changed files with 39 additions and 7 deletions

View File

@ -66,6 +66,7 @@ pub fn get_exn_constructor(
object_id: DefinitionId(class_id),
type_vars: Default::default(),
fields: exception_fields,
static_fields: Default::default(),
methods: vec![("__init__".into(), signature, DefinitionId(cons_id))],
ancestors: vec![
TypeAnnotation::CustomClass { id: DefinitionId(class_id), params: Default::default() },
@ -175,6 +176,7 @@ pub fn get_builtins(primitives: &mut (PrimitiveStore, Unifier)) -> BuiltinInfo {
type_vars: Default::default(),
fields: exception_fields,
methods: Default::default(),
static_fields: Default::default(),
ancestors: vec![],
constructor: None,
resolver: None,
@ -200,6 +202,7 @@ pub fn get_builtins(primitives: &mut (PrimitiveStore, Unifier)) -> BuiltinInfo {
object_id: DefinitionId(10),
type_vars: vec![option_ty_var],
fields: vec![],
static_fields: vec![],
methods: vec![
("is_some".into(), is_some_ty.0, DefinitionId(11)),
("is_none".into(), is_some_ty.0, DefinitionId(12)),

View File

@ -1040,6 +1040,7 @@ impl TopLevelComposer {
class_body_ast,
_class_ancestor_def,
class_fields_def,
class_static_fields_def,
class_methods_def,
class_type_vars_def,
class_resolver,
@ -1047,6 +1048,7 @@ impl TopLevelComposer {
object_id,
ancestors,
fields,
static_fields,
methods,
resolver,
type_vars,
@ -1054,7 +1056,7 @@ impl TopLevelComposer {
} = &mut *class_def
{
if let ast::StmtKind::ClassDef { name, bases, body, .. } = &class_ast {
(*object_id, *name, bases, body, ancestors, fields, methods, type_vars, resolver)
(*object_id, *name, bases, body, ancestors, fields, static_fields, methods, type_vars, resolver)
} else {
unreachable!("here must be class def ast");
}
@ -1268,7 +1270,7 @@ impl TopLevelComposer {
.unify(method_dummy_ty, method_type)
.map_err(|e| e.to_display(unifier).to_string())?;
}
ast::StmtKind::AnnAssign { target, annotation, value: None, .. } => {
ast::StmtKind::AnnAssign { target, annotation, value, .. } => {
Review

What does "reset" mean?
I don't see value being changed in that block of code

What does "reset" mean? I don't see ``value`` being changed in that block of code
if let ast::ExprKind::Name { id: attr, .. } = &target.node {
if defined_fields.insert(attr.to_string()) {
let dummy_field_type = unifier.get_dummy_var().0;
@ -1294,6 +1296,9 @@ impl TopLevelComposer {
_ if core_config.kernel_ann.is_none() => (annotation, true),
_ => continue, // ignore fields annotated otherwise
};
if let Option::Some(..) = &value{
class_static_fields_def.push((*attr, dummy_field_type, mutable));
Review

Why must it?
What happens if it isn't?

Why must it? What happens if it isn't?
}
class_fields_def.push((*attr, dummy_field_type, mutable));
let parsed_annotation = parse_ast_to_type_annotation_kinds(
@ -1324,7 +1329,7 @@ impl TopLevelComposer {
type_var_to_concrete_def.insert(dummy_field_type, parsed_annotation);
} else {
return Err(format!(
"same class fields `{}` defined twice (at {})",
"same class field `{}` defined twice (at {})",
attr, target.location
));
}
@ -1335,7 +1340,27 @@ impl TopLevelComposer {
));
}
}
ast::StmtKind::Assign { .. } => {}, // we don't class attributes
ast::StmtKind::Assign { targets, value, .. } => {
for target in targets {
if let ast::ExprKind::Name { id: attr, .. } = &target.node {
if defined_fields.insert(attr.to_string()) {
Review

"Add assign branch"?
This kind of narration should be in the PR description, not the code comments.

"Add assign branch"? This kind of narration should be in the PR description, not the code comments.
let dummy_field_type = unifier.get_dummy_var().0;
class_static_fields_def.push((*attr, dummy_field_type, true));
class_fields_def.push((*attr, dummy_field_type, true));
} else {
return Err(format!(
"same class field `{}` defined twice (at {})",
attr, target.location
));
}
} else {
return Err(format!(
"unsupported statement type in class definition body (at {})",
Review

style, remove blank line here

style, remove blank line here
target.location
));
}
Review

fields or field?

fields or field?
}
},
ast::StmtKind::Pass { .. } => {}
ast::StmtKind::Expr { value: _, .. } => {} // typically a docstring; ignoring all expressions matches CPython behavior
_ => {
@ -1516,6 +1541,7 @@ impl TopLevelComposer {
ancestors,
methods,
fields,
static_fields,
type_vars,
name: class_name,
object_id,
@ -1618,11 +1644,11 @@ impl TopLevelComposer {
unreachable!("must be init function here")
}
let all_inited = Self::get_all_assigned_field(body.as_slice())?;
for (f, _, _) in fields {
if !all_inited.contains(f) {
for f in fields {
if !all_inited.contains(&f.0) && !static_fields.contains(&f) {
return Err(format!(
"fields `{}` of class `{}` not fully initialized in the initializer (at {})",
f,
&f.0,
class_name,
body[0].location,
));

View File

@ -162,6 +162,7 @@ impl TopLevelComposer {
object_id: DefinitionId(index),
type_vars: Default::default(),
fields: Default::default(),
static_fields: Default::default(),
methods: Default::default(),
ancestors: Default::default(),
constructor,

View File

@ -91,6 +91,8 @@ pub enum TopLevelDef {
// class fields
// name, type, is mutable
fields: Vec<(StrRef, Type, bool)>,
// list of static data members
static_fields: Vec<(StrRef, Type, bool)>,
// class methods, pointing to the corresponding function definition.
methods: Vec<(StrRef, Type, DefinitionId)>,
// ancestor classes, including itself.