[core] typecheck: Disallow redeclaration of var shadowing global

nac3core/src/codegen/expr.rs

@@ -2886,7 +2886,31 @@ pub fn gen_expr<'ctx, G: CodeGenerator>(
             Some((_, Some(static_value), _)) => ValueEnum::Static(static_value.clone()),
             None => {
                 let resolver = ctx.resolver.clone();
-                resolver.get_symbol_value(*id, ctx, generator).unwrap()
+                let value = resolver.get_symbol_value(*id, ctx, generator).unwrap();
+
+                let globals = ctx
+                    .top_level
+                    .definitions
+                    .read()
+                    .iter()
+                    .filter_map(|def| {
+                        if let TopLevelDef::Variable { simple_name, ty, .. } = &*def.read() {
+                            Some((*simple_name, *ty))
+                        } else {
+                            None
+                        }
+                    })
+                    .collect_vec();
+
+                if let Some((_, ty)) = globals.iter().find(|(name, _)| name == id) {
+                    let ptr = value
+                        .to_basic_value_enum(ctx, generator, *ty)
+                        .map(BasicValueEnum::into_pointer_value)?;
+
+                    ctx.builder.build_load(ptr, id.to_string().as_str()).map(Into::into).unwrap()
+                } else {
+                    value
+                }
             }
         },
         ExprKind::List { elts, .. } => {

nac3core/src/toplevel/composer.rs

@@ -485,7 +485,7 @@ impl TopLevelComposer {
                     // things like `class A(Generic[T, V, ImportedModule.T])` is not supported
                     // i.e. only simple names are allowed in the subscript
                     // should update the TopLevelDef::Class.typevars and the TypeEnum::TObj.params
-                    ast::ExprKind::Subscript { value, slice, .. }
+                    ExprKind::Subscript { value, slice, .. }
                         if {
                             matches!(
                                 &value.node,
@@ -501,9 +501,9 @@ impl TopLevelComposer {
                         }
                         is_generic = true;
 
-                        let type_var_list: Vec<&ast::Expr<()>>;
+                        let type_var_list: Vec<&Expr<()>>;
                         // if `class A(Generic[T, V, G])`
-                        if let ast::ExprKind::Tuple { elts, .. } = &slice.node {
+                        if let ExprKind::Tuple { elts, .. } = &slice.node {
                             type_var_list = elts.iter().collect_vec();
                         // `class A(Generic[T])`
                         } else {
@@ -1014,15 +1014,15 @@ impl TopLevelComposer {
                     }
                 }
 
-                let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&ast::Expr>)> =
+                let arg_with_default: Vec<(&ast::Located<ast::ArgData<()>>, Option<&Expr>)> = args
-                    args.args
+                    .args
                     .iter()
                     .rev()
                     .zip(
                         args.defaults
                             .iter()
                             .rev()
-                                .map(|x| -> Option<&ast::Expr> { Some(x) })
+                            .map(|x| -> Option<&Expr> { Some(x) })
                             .chain(std::iter::repeat(None)),
                     )
                     .collect_vec();
@@ -1283,7 +1283,7 @@ impl TopLevelComposer {
 
                         let arg_with_default: Vec<(
                             &ast::Located<ast::ArgData<()>>,
-                            Option<&ast::Expr>,
+                            Option<&Expr>,
                         )> = args
                             .args
                             .iter()
@@ -1292,7 +1292,7 @@ impl TopLevelComposer {
                                 args.defaults
                                     .iter()
                                     .rev()
-                                    .map(|x| -> Option<&ast::Expr> { Some(x) })
+                                    .map(|x| -> Option<&Expr> { Some(x) })
                                     .chain(std::iter::repeat(None)),
                             )
                             .collect_vec();
@@ -1449,7 +1449,7 @@ impl TopLevelComposer {
                         .map_err(|e| HashSet::from([e.to_display(unifier).to_string()]))?;
                 }
                 ast::StmtKind::AnnAssign { target, annotation, value, .. } => {
-                    if let ast::ExprKind::Name { id: attr, .. } = &target.node {
+                    if let ExprKind::Name { id: attr, .. } = &target.node {
                         if defined_fields.insert(attr.to_string()) {
                             let dummy_field_type = unifier.get_dummy_var().ty;
 
@@ -1457,7 +1457,7 @@ impl TopLevelComposer {
                                 None => {
                                     // handle Kernel[T], KernelInvariant[T]
                                     let (annotation, mutable) = match &annotation.node {
-                                        ast::ExprKind::Subscript { value, slice, .. }
+                                        ExprKind::Subscript { value, slice, .. }
                                             if matches!(
                                                 &value.node,
                                                 ast::ExprKind::Name { id, .. } if id == &core_config.kernel_invariant_ann.into()
@@ -1465,7 +1465,7 @@ impl TopLevelComposer {
                                         {
                                             (slice, false)
                                         }
-                                        ast::ExprKind::Subscript { value, slice, .. }
+                                        ExprKind::Subscript { value, slice, .. }
                                             if matches!(
                                                 &value.node,
                                                 ast::ExprKind::Name { id, .. } if core_config.kernel_ann.map_or(false, |c| id == &c.into())
@@ -1483,13 +1483,13 @@ impl TopLevelComposer {
                                 Some(boxed_expr) => {
                                     // Class attributes are set as immutable regardless
                                     let (annotation, _) = match &annotation.node {
-                                        ast::ExprKind::Subscript { slice, .. } => (slice, false),
+                                        ExprKind::Subscript { slice, .. } => (slice, false),
                                         _ if core_config.kernel_ann.is_none() => (annotation, false),
                                         _ => continue,
                                     };
 
                                     match &**boxed_expr {
-                                        ast::Located {location: _, custom: (), node: ast::ExprKind::Constant { value: v, kind: _ }} => {
+                                        ast::Located {location: _, custom: (), node: ExprKind::Constant { value: v, kind: _ }} => {
                                             // Restricting the types allowed to be defined as class attributes
                                             match v {
                                                 ast::Constant::Bool(_) | ast::Constant::Str(_) | ast::Constant::Int(_) | ast::Constant::Float(_) => {}
@@ -1937,20 +1937,20 @@ impl TopLevelComposer {
             if ast.is_none() {
                 return Ok(());
             }
-            let mut function_def = def.write();
+
-            if let TopLevelDef::Function {
+            let (name, simple_name, signature, resolver) = {
-                instance_to_stmt,
+                let function_def = def.read();
-                instance_to_symbol,
+                let TopLevelDef::Function { name, simple_name, signature, resolver, .. } =
-                name,
+                    &*function_def
-                simple_name,
+                else {
-                signature,
+                    return Ok(());
-                resolver,
+                };
-                ..
+
-            } = &mut *function_def
+                (name.clone(), *simple_name, *signature, resolver.clone())
-            {
+            };
-                let signature_ty_enum = unifier.get_ty(*signature);
+
-                let TypeEnum::TFunc(FunSignature { args, ret, vars, .. }) =
+            let signature_ty_enum = unifier.get_ty(signature);
-                    signature_ty_enum.as_ref()
+            let TypeEnum::TFunc(FunSignature { args, ret, vars, .. }) = signature_ty_enum.as_ref()
             else {
                 unreachable!("must be typeenum::tfunc")
             };
@@ -2062,8 +2062,7 @@ impl TopLevelComposer {
                     if self_type.is_some() {
                         result.insert("self".into(), IdentifierInfo::default());
                     }
-                        result
+                    result.extend(inst_args.iter().map(|x| (x.name, IdentifierInfo::default())));
-                            .extend(inst_args.iter().map(|x| (x.name, IdentifierInfo::default())));
                     result
                 };
                 let mut calls: HashMap<CodeLocation, CallId> = HashMap::new();
@@ -2100,33 +2099,43 @@ impl TopLevelComposer {
                 else {
                     unreachable!("must be function def ast")
                 };
-                    if !decorator_list.is_empty()
+
-                        && matches!(&decorator_list[0].node,
-                            ast::ExprKind::Name{ id, .. } if id == &"extern".into())
-                    {
-                        instance_to_symbol.insert(String::new(), simple_name.to_string());
-                        continue;
-                    }
-                    if !decorator_list.is_empty()
-                        && matches!(&decorator_list[0].node,
-                            ast::ExprKind::Name{ id, .. } if id == &"rpc".into())
-                    {
-                        instance_to_symbol.insert(String::new(), simple_name.to_string());
-                        continue;
-                    }
                 if !decorator_list.is_empty() {
-                        if let ast::ExprKind::Call { func, .. } = &decorator_list[0].node {
+                    if matches!(&decorator_list[0].node, ExprKind::Name { id, .. } if id == &"extern".into())
-                            if matches!(&func.node,
-                                ast::ExprKind::Name{ id, .. } if id == &"rpc".into())
                     {
+                        let TopLevelDef::Function { instance_to_symbol, .. } = &mut *def.write()
+                        else {
+                            unreachable!()
+                        };
+                        instance_to_symbol.insert(String::new(), simple_name.to_string());
+                        continue;
+                    }
+
+                    if matches!(&decorator_list[0].node, ExprKind::Name { id, .. } if id == &"rpc".into())
+                    {
+                        let TopLevelDef::Function { instance_to_symbol, .. } = &mut *def.write()
+                        else {
+                            unreachable!()
+                        };
+                        instance_to_symbol.insert(String::new(), simple_name.to_string());
+                        continue;
+                    }
+
+                    if let ExprKind::Call { func, .. } = &decorator_list[0].node {
+                        if matches!(&func.node, ExprKind::Name { id, .. } if id == &"rpc".into()) {
+                            let TopLevelDef::Function { instance_to_symbol, .. } =
+                                &mut *def.write()
+                            else {
+                                unreachable!()
+                            };
                             instance_to_symbol.insert(String::new(), simple_name.to_string());
                             continue;
                         }
                     }
                 }
 
-                    let fun_body = body
+                let fun_body =
-                        .into_iter()
+                    body.into_iter()
                         .map(|b| inferencer.fold_stmt(b))
                         .collect::<Result<Vec<_>, _>>()?;
 
@@ -2196,6 +2205,9 @@ impl TopLevelComposer {
                     )]));
                 }
 
+                let TopLevelDef::Function { instance_to_stmt, .. } = &mut *def.write() else {
+                    unreachable!()
+                };
                 instance_to_stmt.insert(
                     get_subst_key(
                         unifier,
@@ -2211,10 +2223,10 @@ impl TopLevelComposer {
                     },
                 );
             }
-            }
 
             Ok(())
         };
+
         for (id, (def, ast)) in self.definition_ast_list.iter().enumerate().skip(self.builtin_num) {
             if ast.is_none() {
                 continue;

nac3core/src/typecheck/function_check.rs

@@ -10,7 +10,7 @@ use nac3parser::ast::{
 };
 
 use super::{
-    type_inferencer::{IdentifierInfo, Inferencer},
+    type_inferencer::{DeclarationSource, IdentifierInfo, Inferencer},
     typedef::{Type, TypeEnum},
 };
 use crate::toplevel::helper::PrimDef;
@@ -34,6 +34,20 @@ impl<'a> Inferencer<'a> {
                 Err(HashSet::from([format!("cannot assign to a `none` (at {})", pattern.location)]))
             }
             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 matches!(
+                        id_info.source,
+                        DeclarationSource::Global { is_explicit: Some(false) }
+                    ) {
+                        return Err(HashSet::from([format!(
+                            "cannot access local variable '{id}' before it is declared (at {})",
+                            pattern.location
+                        )]));
+                    }
+                }
+
                 if !defined_identifiers.contains_key(id) {
                     defined_identifiers.insert(*id, IdentifierInfo::default());
                 }
@@ -104,7 +118,7 @@ impl<'a> Inferencer<'a> {
                         *id,
                     ) {
                         Ok(_) => {
-                            self.defined_identifiers.insert(*id, IdentifierInfo::default());
+                            defined_identifiers.insert(*id, IdentifierInfo::default());
                         }
                         Err(e) => {
                             return Err(HashSet::from([format!(
@@ -368,9 +382,9 @@ impl<'a> Inferencer<'a> {
             StmtKind::Global { names, .. } => {
                 for id in names {
                     if let Some(id_info) = defined_identifiers.get(id) {
-                        if !id_info.is_global {
+                        if id_info.source == DeclarationSource::Local {
                             return Err(HashSet::from([format!(
-                                "name '{id}' is assigned to before global declaration at {}",
+                                "name '{id}' is used prior to global declaration at {}",
                                 stmt.location,
                             )]));
                         }
@@ -385,8 +399,12 @@ impl<'a> Inferencer<'a> {
                         *id,
                     ) {
                         Ok(_) => {
-                            self.defined_identifiers
+                            self.defined_identifiers.insert(
-                                .insert(*id, IdentifierInfo { is_global: true });
+                                *id,
+                                IdentifierInfo {
+                                    source: DeclarationSource::Global { is_explicit: Some(true) },
+                                },
+                            );
                         }
                         Err(e) => {
                             return Err(HashSet::from([format!(

nac3core/src/typecheck/type_inferencer/mod.rs

@@ -88,11 +88,31 @@ impl PrimitiveStore {
     }
 }
 
+/// The location where an identifier declaration refers to.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum DeclarationSource {
+    /// Local scope.
+    Local,
+
+    /// Global scope.
+    Global {
+        /// Whether the identifier is declared by the use of `global` statement. This field is
+        /// [`None`] if the identifier does not refer to a variable.
+        is_explicit: Option<bool>,
+    },
+}
+
 /// Information regarding a defined identifier.
-#[derive(Clone, Copy, Debug, Default)]
+#[derive(Clone, Copy, Debug)]
 pub struct IdentifierInfo {
     /// Whether this identifier refers to a global variable.
-    pub is_global: bool,
+    pub source: DeclarationSource,
+}
+
+impl Default for IdentifierInfo {
+    fn default() -> Self {
+        IdentifierInfo { source: DeclarationSource::Local }
+    }
 }
 
 impl IdentifierInfo {
@@ -566,15 +586,45 @@ impl<'a> Fold<()> for Inferencer<'a> {
                         unreachable!("must be tobj")
                     }
                 } else {
+                    let top_level_defs = &self.top_level.definitions.read();
+
                     if !self.defined_identifiers.contains_key(id) {
                         match self.function_data.resolver.get_symbol_type(
                             self.unifier,
-                            &self.top_level.definitions.read(),
+                            top_level_defs,
                             self.primitives,
                             *id,
                         ) {
                             Ok(_) => {
-                                self.defined_identifiers.insert(*id, IdentifierInfo::default());
+                                // Determine if the referenced id refers to a global symbol
+                                let is_explicit = top_level_defs
+                                    .iter()
+                                    .map(|def| match *def.read() {
+                                        TopLevelDef::Class { name, .. } => (name, false),
+                                        TopLevelDef::Function { simple_name, .. } => {
+                                            (simple_name, false)
+                                        }
+                                        TopLevelDef::Variable { simple_name, .. } => {
+                                            (simple_name, true)
+                                        }
+                                    })
+                                    .find(|(global, _)| global == id)
+                                    .map(|(_, has_explicit_prop)| has_explicit_prop);
+
+                                self.defined_identifiers.insert(
+                                    *id,
+                                    IdentifierInfo {
+                                        source: match is_explicit {
+                                            Some(true) => DeclarationSource::Global {
+                                                is_explicit: Some(false),
+                                            },
+                                            Some(false) => {
+                                                DeclarationSource::Global { is_explicit: None }
+                                            }
+                                            None => DeclarationSource::Local,
+                                        },
+                                    },
+                                );
                             }
                             Err(e) => {
                                 return report_error(