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,
Outdated
Review

What function?
Comment is incomprehensible.

What function? Comment is incomprehensible.
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(),
Outdated
Review

Is this any different than the other lines in this block of code, which don't have a comment?

Is this any different than the other lines in this block of code, which don't have a comment?
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.
Outdated
Review

Misplaced comment.

Misplaced comment.
methods: Vec<(StrRef, Type, DefinitionId)>,
// ancestor classes, including itself.