From 71308ae5d48788b975856cec6d596077866bda10 Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 14:25:41 +0800 Subject: [PATCH 1/9] nac3core: Add field constructor_lookup(HashMap) to the TopLevelComposer and the function build_constructor_lookup to build the HashMap --- nac3artiq/src/lib.rs | 2 +- nac3core/src/toplevel/composer.rs | 144 +++++++++++++++++++++++++++++- nac3standalone/src/main.rs | 1 + 3 files changed, 142 insertions(+), 5 deletions(-) diff --git a/nac3artiq/src/lib.rs b/nac3artiq/src/lib.rs index 4417a682..70214d54 100644 --- a/nac3artiq/src/lib.rs +++ b/nac3artiq/src/lib.rs @@ -280,7 +280,7 @@ impl Nac3 { self.builtins.clone(), ComposerConfig { kernel_ann: Some("Kernel"), kernel_invariant_ann: "KernelInvariant" }, ); - + composer.build_constructor_lookup(self.top_levels.iter().map(|(stmt, _, _)| stmt)); let builtins = PyModule::import(py, "builtins")?; let typings = PyModule::import(py, "typing")?; let id_fn = builtins.getattr("id")?; diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index 16f508a7..98914f38 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -37,6 +37,8 @@ pub struct TopLevelComposer { // number of built-in function and classes in the definition list, later skip pub builtin_num: usize, pub core_config: ComposerConfig, + // the HashMap that store the class name and It constructor Function + pub constructor_lookup: HashMap>, } impl Default for TopLevelComposer { @@ -132,12 +134,145 @@ impl TopLevelComposer { defined_names, method_class, core_config, + constructor_lookup: Default::default(), }, builtin_id, builtin_ty, ) } + pub fn build_constructor_lookup<'a, I>(&mut self, stmts: I) + where + I: Iterator> + { + let classes = Vec::from_iter(stmts.filter_map(|stmt| { + if let ast::StmtKind::ClassDef { name, bases, body, .. } = &stmt.node { + Some((name, bases, body)) + } else { + None + } + })); + + let base_class_lookup: HashMap = HashMap::from_iter( + classes.iter().filter_map(|(class_name, bases, _)| { + // Get the first base class in the Vector of bases is good enough, since we only support single inheritance + let base_class = bases.get(0).and_then(|ast::Located { node, .. }| { + if let ast::ExprKind::Name { id, .. } = node { + Some(*id) + } else { + None + } + }); + + if let Some(base_class) = base_class { + Some((**class_name, base_class)) + } else { + None + } + }) + ); + + let mut constructor_lookup: HashMap> = HashMap::from_iter( + classes.iter().filter_map(|(class_name, _, body)| { + for stmt in body.iter() { + let func_name = if let ast::StmtKind::FunctionDef { name, .. } = &stmt.node { + name + } else { + continue; + }; + + if func_name == &"__init__".into() { + return Some((**class_name, stmt.clone())) + } + } + return None + }) + ); + + constructor_lookup.insert( + "Exception".into(), + ast::Located::new( + ast::Location::new(0, 0, ast::FileName("".into())), + ast::StmtKind::FunctionDef { + name: "__init__".into(), + args: Box::new(ast::Arguments { + posonlyargs: vec![], + args: vec![ + ast::Located { + location: Location { + row: 8, + column: 18, + file: ast::FileName("".into()), + }, + custom: (), + node: ast::ArgData { + arg: "self".into(), + annotation: None, + type_comment: None, + }, + }, + ast::Located { + location: Location { + row: 8, + column: 18, + file: ast::FileName("".into()), + }, + custom: (), + node: ast::ArgData { + arg: "message".into(), + annotation: None, + type_comment: None, + }, + } + ], + vararg: None, + kwonlyargs: Default::default(), + kw_defaults: Default::default(), + kwarg: None, + defaults: Default::default(), + }), + body: vec![ + ast::Located { + location: Location { + row: 0, + column: 0, + file: ast::FileName("".into()), + }, + custom: (), + node: ast::StmtKind::Pass { + config_comment: Default::default(), + }, + }, + ], + decorator_list: Default::default(), + returns: None, + type_comment: Default::default(), + config_comment: Default::default(), + } + ) + ); + + for (class, _, _) in classes + .iter() + .filter(|(c, _, _)| constructor_lookup.get(c).is_none()) + { + let mut current_class = class.clone(); + while let Some(base) = base_class_lookup.get(current_class) { + if let Some(cons) = constructor_lookup.get(base) { + self.constructor_lookup.insert(*current_class, cons.clone()); + break; + } else { + current_class = base; + } + } + } + + // copy the rest of the classes with constructor into the self.constructor_lookup + for (k, v) in constructor_lookup.into_iter() { + self.constructor_lookup.insert(k, v); + } + } + pub fn make_top_level_context(&self) -> TopLevelContext { TopLevelContext { definitions: RwLock::new( @@ -222,18 +357,19 @@ impl TopLevelComposer { // we do not push anything to the def list, so we keep track of the index // and then push in the correct order after the for loop let mut class_method_index_offset = 0; - let init_id = "__init__".into(); let exception_id = "Exception".into(); // TODO: Fix this hack. We will generate constructor for classes that inherit // from Exception class (directly or indirectly), but this code cannot handle // subclass of other exception classes. let mut contains_constructor = bases .iter().any(|base| matches!(base.node, ast::ExprKind::Name { id, .. } if id == exception_id)); + + if self.constructor_lookup.contains_key(&class_name) { + contains_constructor = true; + } + for b in body { if let ast::StmtKind::FunctionDef { name: method_name, .. } = &b.node { - if method_name == &init_id { - contains_constructor = true; - } if self.keyword_list.contains(method_name) { return Err(format!( "cannot use keyword `{}` as a method name (at {})", diff --git a/nac3standalone/src/main.rs b/nac3standalone/src/main.rs index cc50fb63..703310a1 100644 --- a/nac3standalone/src/main.rs +++ b/nac3standalone/src/main.rs @@ -183,6 +183,7 @@ fn main() { Arc::new(Resolver(internal_resolver.clone())) as Arc; let parser_result = parser::parse_program(&program, file_name.into()).unwrap(); + composer.build_constructor_lookup(parser_result.iter()); for stmt in parser_result.into_iter() { match &stmt.node { -- 2.47.2 From 7d68efc59902e70cb746ad7368d6d5109272d035 Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 14:29:26 +0800 Subject: [PATCH 2/9] nac3core: Add field constructor_lookup(HashMap) to the TopLevelComposer and the function build_constructor_lookup to build the HashMap --- nac3core/src/toplevel/composer.rs | 65 +------------------------------ 1 file changed, 1 insertion(+), 64 deletions(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index 98914f38..885173c2 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -172,7 +172,7 @@ impl TopLevelComposer { }) ); - let mut constructor_lookup: HashMap> = HashMap::from_iter( + let constructor_lookup: HashMap> = HashMap::from_iter( classes.iter().filter_map(|(class_name, _, body)| { for stmt in body.iter() { let func_name = if let ast::StmtKind::FunctionDef { name, .. } = &stmt.node { @@ -189,69 +189,6 @@ impl TopLevelComposer { }) ); - constructor_lookup.insert( - "Exception".into(), - ast::Located::new( - ast::Location::new(0, 0, ast::FileName("".into())), - ast::StmtKind::FunctionDef { - name: "__init__".into(), - args: Box::new(ast::Arguments { - posonlyargs: vec![], - args: vec![ - ast::Located { - location: Location { - row: 8, - column: 18, - file: ast::FileName("".into()), - }, - custom: (), - node: ast::ArgData { - arg: "self".into(), - annotation: None, - type_comment: None, - }, - }, - ast::Located { - location: Location { - row: 8, - column: 18, - file: ast::FileName("".into()), - }, - custom: (), - node: ast::ArgData { - arg: "message".into(), - annotation: None, - type_comment: None, - }, - } - ], - vararg: None, - kwonlyargs: Default::default(), - kw_defaults: Default::default(), - kwarg: None, - defaults: Default::default(), - }), - body: vec![ - ast::Located { - location: Location { - row: 0, - column: 0, - file: ast::FileName("".into()), - }, - custom: (), - node: ast::StmtKind::Pass { - config_comment: Default::default(), - }, - }, - ], - decorator_list: Default::default(), - returns: None, - type_comment: Default::default(), - config_comment: Default::default(), - } - ) - ); - for (class, _, _) in classes .iter() .filter(|(c, _, _)| constructor_lookup.get(c).is_none()) -- 2.47.2 From a6bd6d5567964836fbbdc7c7b7c5e118d3d6fd28 Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 16:49:04 +0800 Subject: [PATCH 3/9] nac3core: src/toplevel/composer.rs fix the constructor propagation bug in function build_constructor_lookup --- nac3core/src/toplevel/composer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index 885173c2..570cac1d 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -196,7 +196,7 @@ impl TopLevelComposer { let mut current_class = class.clone(); while let Some(base) = base_class_lookup.get(current_class) { if let Some(cons) = constructor_lookup.get(base) { - self.constructor_lookup.insert(*current_class, cons.clone()); + self.constructor_lookup.insert(**class, cons.clone()); break; } else { current_class = base; -- 2.47.2 From 510eaae73ac1e0169c57375cd432d89d2ad5e487 Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 16:50:09 +0800 Subject: [PATCH 4/9] nac3core: Add test for toplevel::composer::build_constructor_lookup --- nac3core/src/toplevel/test.rs | 145 ++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/nac3core/src/toplevel/test.rs b/nac3core/src/toplevel/test.rs index 49400711..6ad6fceb 100644 --- a/nac3core/src/toplevel/test.rs +++ b/nac3core/src/toplevel/test.rs @@ -566,6 +566,151 @@ fn test_analyze(source: Vec<&str>, res: Vec<&str>) { } } +#[test_case( + indoc! {" + class A: + def __init__(self): + pass + + class B(A): + pass + + class C(B): + pass + "}, + HashMap::from([ + ("A".into(),ast::Located { + location: ast::Location::new(2, 5, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::FunctionDef { + name: "__init__".into(), + args: Box::new(ast::Arguments { + posonlyargs: vec![], + args: vec![ + ast::Located { + location: ast::Location::new(2, 18, ast::FileName("unknown".into())), + custom: (), + node: ast::ArgData { + arg: "self".into(), + annotation: None, + type_comment: None, + }, + }, + ], + vararg: None, + kwonlyargs: vec![], + kw_defaults: vec![], + kwarg: None, + defaults: vec![], + }), + body: vec![ + ast::Located { + location: Location::new(3, 9, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::Pass { + config_comment: vec![], + }, + }, + ], + decorator_list: vec![], + returns: None, + type_comment: None, + config_comment: vec![], + }, + }), + ("B".into(), ast::Located { + location: ast::Location::new(2, 5, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::FunctionDef { + name: "__init__".into(), + args: Box::new(ast::Arguments { + posonlyargs: vec![], + args: vec![ + ast::Located { + location: ast::Location::new(2, 18, ast::FileName("unknown".into())), + custom: (), + node: ast::ArgData { + arg: "self".into(), + annotation: None, + type_comment: None, + }, + }, + ], + vararg: None, + kwonlyargs: vec![], + kw_defaults: vec![], + kwarg: None, + defaults: vec![], + }), + body: vec![ + ast::Located { + location: Location::new(3, 9, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::Pass { + config_comment: vec![], + }, + }, + ], + decorator_list: vec![], + returns: None, + type_comment: None, + config_comment: vec![], + }, + }), + ("C".into(), ast::Located { + location: ast::Location::new(2, 5, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::FunctionDef { + name: "__init__".into(), + args: Box::new(ast::Arguments { + posonlyargs: vec![], + args: vec![ + ast::Located { + location: ast::Location::new(2, 18, ast::FileName("unknown".into())), + custom: (), + node: ast::ArgData { + arg: "self".into(), + annotation: None, + type_comment: None, + }, + }, + ], + vararg: None, + kwonlyargs: vec![], + kw_defaults: vec![], + kwarg: None, + defaults: vec![], + }), + body: vec![ + ast::Located { + location: Location::new(3, 9, ast::FileName("unknown".into())), + custom: (), + node: ast::StmtKind::Pass { + config_comment: vec![], + }, + }, + ], + decorator_list: vec![], + returns: None, + type_comment: None, + config_comment: vec![], + }, + }), + ]); + "build_constructor_lookup_table" +)] +fn test_build_constructor_lookup(source: &str, result: HashMap>) { + let ast = parse_program(source, Default::default()); + let mut composer: TopLevelComposer = Default::default(); + if let Ok(stmts) = ast { + composer.build_constructor_lookup(stmts.iter()) + } + + assert_eq!(result.get(&"A".into()).unwrap(), result.get(&"B".into()).unwrap()); + assert_eq!(result.get(&"B".into()).unwrap(), result.get(&"C".into()).unwrap()); +} + + #[test_case( vec![ indoc! {" -- 2.47.2 From ad12b7e958a3e780ce30c6e0c3089044e3a2179a Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 16:56:41 +0800 Subject: [PATCH 5/9] update comment --- nac3artiq/src/lib.rs | 1 + nac3core/src/toplevel/composer.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/nac3artiq/src/lib.rs b/nac3artiq/src/lib.rs index 70214d54..2ede61a6 100644 --- a/nac3artiq/src/lib.rs +++ b/nac3artiq/src/lib.rs @@ -281,6 +281,7 @@ impl Nac3 { ComposerConfig { kernel_ann: Some("Kernel"), kernel_invariant_ann: "KernelInvariant" }, ); composer.build_constructor_lookup(self.top_levels.iter().map(|(stmt, _, _)| stmt)); + let builtins = PyModule::import(py, "builtins")?; let typings = PyModule::import(py, "typing")?; let id_fn = builtins.getattr("id")?; diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index 570cac1d..cc833389 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -37,7 +37,7 @@ pub struct TopLevelComposer { // number of built-in function and classes in the definition list, later skip pub builtin_num: usize, pub core_config: ComposerConfig, - // the HashMap that store the class name and It constructor Function + // the HashMap that store the class name and it's constructor function pub constructor_lookup: HashMap>, } -- 2.47.2 From c43be7388d4e2a07150ddc808deb1319935c13ad Mon Sep 17 00:00:00 2001 From: z78078 Date: Wed, 20 Jul 2022 16:58:04 +0800 Subject: [PATCH 6/9] nac3core: update comment --- nac3core/src/toplevel/composer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index cc833389..31d0cf04 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -37,7 +37,7 @@ pub struct TopLevelComposer { // number of built-in function and classes in the definition list, later skip pub builtin_num: usize, pub core_config: ComposerConfig, - // the HashMap that store the class name and it's constructor function + // the class name and it's constructor function pub constructor_lookup: HashMap>, } -- 2.47.2 From 6eb433b2e35dc27b0b97a74a2c350706b41e54e9 Mon Sep 17 00:00:00 2001 From: z78078 Date: Thu, 21 Jul 2022 09:44:05 +0800 Subject: [PATCH 7/9] nac3core: fix typo and remove the unnecessary 'return' statement --- nac3core/src/toplevel/composer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index 31d0cf04..bf72f98f 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -37,7 +37,7 @@ pub struct TopLevelComposer { // number of built-in function and classes in the definition list, later skip pub builtin_num: usize, pub core_config: ComposerConfig, - // the class name and it's constructor function + // the class name and its constructor function pub constructor_lookup: HashMap>, } @@ -185,7 +185,7 @@ impl TopLevelComposer { return Some((**class_name, stmt.clone())) } } - return None + None }) ); -- 2.47.2 From 2ec449545e3bdef2c9f54d6a67c5a069d8847240 Mon Sep 17 00:00:00 2001 From: z78078 Date: Thu, 21 Jul 2022 09:56:58 +0800 Subject: [PATCH 8/9] nac3core: fix test assertion --- nac3core/src/toplevel/test.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nac3core/src/toplevel/test.rs b/nac3core/src/toplevel/test.rs index 6ad6fceb..1fa35a30 100644 --- a/nac3core/src/toplevel/test.rs +++ b/nac3core/src/toplevel/test.rs @@ -706,8 +706,9 @@ fn test_build_constructor_lookup(source: &str, result: HashMap Date: Thu, 21 Jul 2022 15:39:03 +0800 Subject: [PATCH 9/9] nac3core: coding style change --- nac3core/src/toplevel/composer.rs | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index bf72f98f..c11855b9 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -156,36 +156,29 @@ impl TopLevelComposer { let base_class_lookup: HashMap = HashMap::from_iter( classes.iter().filter_map(|(class_name, bases, _)| { // Get the first base class in the Vector of bases is good enough, since we only support single inheritance - let base_class = bases.get(0).and_then(|ast::Located { node, .. }| { + bases + .get(0) + .and_then(|ast::Located { node, .. }| { if let ast::ExprKind::Name { id, .. } = node { Some(*id) } else { None } - }); - - if let Some(base_class) = base_class { - Some((**class_name, base_class)) - } else { - None - } + }) + .and_then(|base_class_name| Some((**class_name, base_class_name))) }) ); let constructor_lookup: HashMap> = HashMap::from_iter( classes.iter().filter_map(|(class_name, _, body)| { - for stmt in body.iter() { - let func_name = if let ast::StmtKind::FunctionDef { name, .. } = &stmt.node { - name - } else { - continue; - }; - - if func_name == &"__init__".into() { - return Some((**class_name, stmt.clone())) + body.iter().find_map(|stmt| { + if let ast::StmtKind::FunctionDef { name, .. } = &stmt.node { + if name == &"__init__".into() { + return Some((**class_name, stmt.clone())); + } } - } - None + None + }) }) ); -- 2.47.2