From 9c6685fa8fb50555b98901b224a935878659e95d Mon Sep 17 00:00:00 2001 From: David Mak Date: Mon, 7 Oct 2024 16:51:37 +0800 Subject: [PATCH 1/4] [core] typecheck/function_check: Fix lookup of defined ids in scope --- nac3core/src/typecheck/function_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nac3core/src/typecheck/function_check.rs b/nac3core/src/typecheck/function_check.rs index 0893adec..ed801a17 100644 --- a/nac3core/src/typecheck/function_check.rs +++ b/nac3core/src/typecheck/function_check.rs @@ -36,7 +36,7 @@ impl<'a> Inferencer<'a> { ExprKind::Name { id, .. } => { // If `id` refers to a declared symbol, reject this assignment if it is used in the // context of an (implicit) global variable - if let Some(id_info) = self.defined_identifiers.get(id) { + if let Some(id_info) = defined_identifiers.get(id) { if matches!( id_info.source, DeclarationSource::Global { is_explicit: Some(false) } -- 2.44.2 From 65a12d9ab326a23f27e995b3789cb7679584a482 Mon Sep 17 00:00:00 2001 From: David Mak Date: Mon, 7 Oct 2024 16:52:39 +0800 Subject: [PATCH 2/4] [core] Refactor registration of top-level variables --- nac3core/src/toplevel/composer.rs | 137 ++++++++++++++++-------------- nac3core/src/toplevel/helper.rs | 2 +- nac3core/src/toplevel/mod.rs | 4 +- nac3standalone/src/main.rs | 17 ++-- 4 files changed, 87 insertions(+), 73 deletions(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index ab5e4d80..f552229f 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use nac3parser::ast::{fold::Fold, ExprKind}; +use nac3parser::ast::{fold::Fold, ExprKind, Ident}; use super::*; use crate::{ @@ -386,50 +386,17 @@ impl TopLevelComposer { let ExprKind::Name { id: name, .. } = target.node else { return Err(format!( "global variable declaration must be an identifier (at {})", - ast.location + target.location )); }; - if self.keyword_list.contains(&name) { - return Err(format!( - "cannot use keyword `{}` as a class name (at {})", - name, - ast.location - )); - } - - let global_var_name = if mod_path.is_empty() { - name.to_string() - } else { - format!("{mod_path}.{name}") - }; - if !defined_names.insert(global_var_name.clone()) { - return Err(format!( - "global variable `{}` defined twice (at {})", - global_var_name, - ast.location - )); - } - - let ty_to_be_unified = self.unifier.get_dummy_var().ty; - self.definition_ast_list.push(( - RwLock::new(Self::make_top_level_variable_def( - global_var_name, - name, - // dummy here, unify with correct type later, - ty_to_be_unified, - *(annotation.clone()), - resolver, - Some(ast.location), - )).into(), - None, - )); - - Ok(( + self.register_top_level_var( name, - DefinitionId(self.definition_ast_list.len() - 1), - Some(ty_to_be_unified), - )) + Some(annotation.as_ref().clone()), + resolver, + mod_path, + target.location, + ) } _ => Err(format!( @@ -439,6 +406,50 @@ impl TopLevelComposer { } } + /// Registers a top-level variable with the given `name` into the composer. + /// + /// `annotation` - The type annotation of the top-level variable, or [`None`] if no type + /// annotation is provided. + /// `location` - The location of the top-level variable. + pub fn register_top_level_var( + &mut self, + name: Ident, + annotation: Option, + resolver: Option>, + mod_path: &str, + location: Location, + ) -> Result<(StrRef, DefinitionId, Option), String> { + if self.keyword_list.contains(&name) { + return Err(format!("cannot use keyword `{name}` as a class name (at {location})")); + } + + let global_var_name = + if mod_path.is_empty() { name.to_string() } else { format!("{mod_path}.{name}") }; + + if !self.defined_names.insert(global_var_name.clone()) { + return Err(format!( + "global variable `{global_var_name}` defined twice (at {location})" + )); + } + + let ty_to_be_unified = self.unifier.get_dummy_var().ty; + self.definition_ast_list.push(( + RwLock::new(Self::make_top_level_variable_def( + global_var_name, + name, + // dummy here, unify with correct type later, + ty_to_be_unified, + annotation, + resolver, + Some(location), + )) + .into(), + None, + )); + + Ok((name, DefinitionId(self.definition_ast_list.len() - 1), Some(ty_to_be_unified))) + } + pub fn start_analysis(&mut self, inference: bool) -> Result<(), HashSet> { self.analyze_top_level_class_type_var()?; self.analyze_top_level_class_bases()?; @@ -2249,9 +2260,8 @@ impl TopLevelComposer { let primitives_store = &self.primitives_ty; let mut analyze = |variable_def: &Arc>| -> Result<_, HashSet> { - let variable_def = &mut *variable_def.write(); - - let TopLevelDef::Variable { ty: dummy_ty, ty_decl, resolver, loc, .. } = variable_def + let TopLevelDef::Variable { ty: dummy_ty, ty_decl, resolver, loc, .. } = + &*variable_def.read() else { // not top level variable def, skip return Ok(()); @@ -2259,25 +2269,28 @@ impl TopLevelComposer { let resolver = &**resolver.as_ref().unwrap(); - let ty_annotation = parse_ast_to_type_annotation_kinds( - resolver, - &temp_def_list, - unifier, - primitives_store, - ty_decl, - HashMap::new(), - )?; - let ty_from_ty_annotation = get_type_from_type_annotation_kinds( - &temp_def_list, - unifier, - primitives_store, - &ty_annotation, - &mut None, - )?; + if let Some(ty_decl) = ty_decl { + let ty_annotation = parse_ast_to_type_annotation_kinds( + resolver, + &temp_def_list, + unifier, + primitives_store, + ty_decl, + HashMap::new(), + )?; + let ty_from_ty_annotation = get_type_from_type_annotation_kinds( + &temp_def_list, + unifier, + primitives_store, + &ty_annotation, + &mut None, + )?; + + unifier.unify(*dummy_ty, ty_from_ty_annotation).map_err(|e| { + HashSet::from([e.at(Some(loc.unwrap())).to_display(unifier).to_string()]) + })?; + } - unifier.unify(*dummy_ty, ty_from_ty_annotation).map_err(|e| { - HashSet::from([e.at(Some(loc.unwrap())).to_display(unifier).to_string()]) - })?; Ok(()) }; diff --git a/nac3core/src/toplevel/helper.rs b/nac3core/src/toplevel/helper.rs index d674c51b..f2948754 100644 --- a/nac3core/src/toplevel/helper.rs +++ b/nac3core/src/toplevel/helper.rs @@ -600,7 +600,7 @@ impl TopLevelComposer { name: String, simple_name: StrRef, ty: Type, - ty_decl: Expr, + ty_decl: Option, resolver: Option>, loc: Option, ) -> TopLevelDef { diff --git a/nac3core/src/toplevel/mod.rs b/nac3core/src/toplevel/mod.rs index 8241c3ac..e0479a88 100644 --- a/nac3core/src/toplevel/mod.rs +++ b/nac3core/src/toplevel/mod.rs @@ -158,8 +158,8 @@ pub enum TopLevelDef { /// Type of the global variable. ty: Type, - /// The declared type of the global variable. - ty_decl: Expr, + /// The declared type of the global variable, or [`None`] if no type annotation is provided. + ty_decl: Option, /// Symbol resolver of the module defined the class. resolver: Option>, diff --git a/nac3standalone/src/main.rs b/nac3standalone/src/main.rs index e27c5e1f..9b1a601c 100644 --- a/nac3standalone/src/main.rs +++ b/nac3standalone/src/main.rs @@ -248,8 +248,9 @@ fn handle_assignment_pattern( fn handle_global_var( target: &Expr, value: Option<&Expr>, - resolver: &(dyn SymbolResolver + Send + Sync), + resolver: &Arc, internal_resolver: &ResolverInternal, + composer: &mut TopLevelComposer, ) -> Result<(), String> { let ExprKind::Name { id, .. } = target.node else { return Err(format!( @@ -262,8 +263,12 @@ fn handle_global_var( return Err(format!("global variable `{id}` must be initialized in its definition")); }; - if let Ok(val) = parse_parameter_default_value(value, resolver) { + if let Ok(val) = parse_parameter_default_value(value, &**resolver) { internal_resolver.add_module_global(id, val); + let (name, def_id, _) = composer + .register_top_level_var(id, None, Some(resolver.clone()), "__main__", target.location) + .unwrap(); + internal_resolver.add_id_def(name, def_id); Ok(()) } else { Err(format!( @@ -375,16 +380,12 @@ fn main() { if let Err(err) = handle_global_var( target, value.as_ref().map(Box::as_ref), - resolver.as_ref(), + &resolver, internal_resolver.as_ref(), + &mut composer, ) { panic!("{err}"); } - - let (name, def_id, _) = composer - .register_top_level(stmt, Some(resolver.clone()), "__main__", true) - .unwrap(); - internal_resolver.add_id_def(name, def_id); } // allow (and ignore) "from __future__ import annotations" -- 2.44.2 From 56c845aac4548f16a15b6ebbeae48ab7e9aa1caa Mon Sep 17 00:00:00 2001 From: David Mak Date: Mon, 7 Oct 2024 17:00:20 +0800 Subject: [PATCH 3/4] [standalone] Add support for registering globals without type decl --- nac3core/src/toplevel/composer.rs | 13 +++++++ nac3standalone/src/main.rs | 62 +++++++++++++++---------------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/nac3core/src/toplevel/composer.rs b/nac3core/src/toplevel/composer.rs index f552229f..b93e1b36 100644 --- a/nac3core/src/toplevel/composer.rs +++ b/nac3core/src/toplevel/composer.rs @@ -382,6 +382,19 @@ impl TopLevelComposer { )) } + ast::StmtKind::Assign { .. } => { + // Assignment statements can assign to (and therefore create) more than one + // variable, but this function only allows returning one set of symbol information. + // We want to avoid changing this to return a `Vec` of symbol info, as this would + // require `iter().next().unwrap()` on every variable created from a non-Assign + // statement. + // + // Make callers use `register_top_level_var` instead, as it provides more + // fine-grained control over which symbols to register, while also simplifying the + // usage of this function. + panic!("Registration of top-level Assign statements must use TopLevelComposer::register_top_level_var (at {})", ast.location); + } + ast::StmtKind::AnnAssign { target, annotation, .. } => { let ExprKind::Name { id: name, .. } = target.node else { return Err(format!( diff --git a/nac3standalone/src/main.rs b/nac3standalone/src/main.rs index 9b1a601c..71bfe8e5 100644 --- a/nac3standalone/src/main.rs +++ b/nac3standalone/src/main.rs @@ -174,46 +174,49 @@ fn handle_typevar_definition( fn handle_assignment_pattern( targets: &[Expr], value: &Expr, - resolver: &(dyn SymbolResolver + Send + Sync), + resolver: Arc, internal_resolver: &ResolverInternal, - def_list: &[Arc>], - unifier: &mut Unifier, - primitives: &PrimitiveStore, + composer: &mut TopLevelComposer, ) -> Result<(), String> { if targets.len() == 1 { - match &targets[0].node { + let target = &targets[0]; + + match &target.node { ExprKind::Name { id, .. } => { + let def_list = composer.extract_def_list(); + let unifier = &mut composer.unifier; + let primitives = &composer.primitives_ty; + if let Ok(var) = - handle_typevar_definition(value, resolver, def_list, unifier, primitives) + handle_typevar_definition(value, &*resolver, &def_list, unifier, primitives) { internal_resolver.add_id_type(*id, var); Ok(()) - } else if let Ok(val) = parse_parameter_default_value(value, resolver) { + } else if let Ok(val) = parse_parameter_default_value(value, &*resolver) { internal_resolver.add_module_global(*id, val); + let (name, def_id, _) = composer + .register_top_level_var( + *id, + None, + Some(resolver.clone()), + "__main__", + target.location, + ) + .unwrap(); + internal_resolver.add_id_def(name, def_id); Ok(()) } else { Err(format!("fails to evaluate this expression `{:?}` as a constant or generic parameter at {}", - targets[0].node, - targets[0].location, + target.node, + target.location, )) } } ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } => { - handle_assignment_pattern( - elts, - value, - resolver, - internal_resolver, - def_list, - unifier, - primitives, - )?; + handle_assignment_pattern(elts, value, resolver, internal_resolver, composer)?; Ok(()) } - _ => Err(format!( - "assignment to {:?} is not supported at {}", - targets[0], targets[0].location - )), + _ => Err(format!("assignment to {target:?} is not supported at {}", target.location)), } } else { match &value.node { @@ -223,11 +226,9 @@ fn handle_assignment_pattern( handle_assignment_pattern( std::slice::from_ref(tar), val, - resolver, + resolver.clone(), internal_resolver, - def_list, - unifier, - primitives, + composer, )?; } Ok(()) @@ -360,17 +361,12 @@ fn main() { for stmt in parser_result { match &stmt.node { StmtKind::Assign { targets, value, .. } => { - let def_list = composer.extract_def_list(); - let unifier = &mut composer.unifier; - let primitives = &composer.primitives_ty; if let Err(err) = handle_assignment_pattern( targets, value, - resolver.as_ref(), + resolver.clone(), internal_resolver.as_ref(), - &def_list, - unifier, - primitives, + &mut composer, ) { panic!("{err}"); } -- 2.44.2 From 5839badadd732262dc24af2971a94dde60f7103e Mon Sep 17 00:00:00 2001 From: David Mak Date: Mon, 7 Oct 2024 17:00:45 +0800 Subject: [PATCH 4/4] [standalone] Update globals.py with type-inferred global var --- nac3standalone/demo/src/globals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nac3standalone/demo/src/globals.py b/nac3standalone/demo/src/globals.py index d8b7822d..aa3d72e2 100644 --- a/nac3standalone/demo/src/globals.py +++ b/nac3standalone/demo/src/globals.py @@ -7,7 +7,7 @@ def output_int64(x: int64): ... X: int32 = 0 -Y: int64 = int64(1) +Y = int64(1) def f(): global X, Y -- 2.44.2