fix review feedbacks

crates/analyzer/src/analyzer_error.rs

@@ -227,6 +227,21 @@ pub enum AnalyzerError {
         error_location: SourceSpan,
     },
 
+    #[diagnostic(
+        severity(Error),
+        code(missing_default_argument),
+        help("give default argument"),
+        url("")
+    )]
+    #[error("missing default argument for parameter {identifier}")]
+    MissingDefaultArgument {
+        identifier: String,
+        #[source_code]
+        input: NamedSource,
+        #[label("Error location")]
+        error_location: SourceSpan,
+    },
+
     #[diagnostic(
         severity(Error),
         code(mismatch_function_arity),
@@ -840,6 +855,14 @@ impl AnalyzerError {
         }
     }
 
+    pub fn missing_default_argument(identifier: &str, source: &str, token: &TokenRange) -> Self {
+        AnalyzerError::MissingDefaultArgument {
+            identifier: identifier.into(),
+            input: AnalyzerError::named_source(source, token),
+            error_location: token.into(),
+        }
+    }
+
     pub fn mismatch_function_arity(
         name: &str,
         arity: usize,

crates/analyzer/src/handlers/create_reference.rs

@@ -1,12 +1,12 @@
 use crate::analyzer_error::AnalyzerError;
 use crate::namespace::Namespace;
 use crate::namespace_table;
-use crate::symbol::{DocComment, GenericInstanceProperty, Symbol, SymbolKind};
-use crate::symbol_path::{GenericSymbol, GenericSymbolPath, SymbolPath};
+use crate::symbol::SymbolKind;
+use crate::symbol_path::{GenericSymbolPath, SymbolPath};
 use crate::symbol_table::{self, ResolveError, ResolveErrorCause};
 use veryl_parser::resource_table::TokenId;
 use veryl_parser::veryl_grammar_trait::*;
-use veryl_parser::veryl_token::{Token, TokenRange};
+use veryl_parser::veryl_token::TokenRange;
 use veryl_parser::veryl_walker::{Handler, HandlerPoint};
 use veryl_parser::ParolError;
 
@@ -92,9 +92,7 @@ impl<'a> CreateReference<'a> {
                         path.arguments.push(param.1.as_ref().unwrap().clone());
                     }
 
-                    if let Some((token, new_symbol)) =
-                        self.get_generic_instance(&symbol.found, &path)
-                    {
+                    if let Some((token, new_symbol)) = path.get_generic_instance(&symbol.found) {
                         if let Some(ref x) = symbol_table::insert(&token, new_symbol) {
                             symbol_table::add_generic_instance(symbol.found.id, *x);
                         }
@@ -119,29 +117,6 @@ impl<'a> CreateReference<'a> {
             }
         }
     }
-
-    fn get_generic_instance(&self, base: &Symbol, path: &GenericSymbol) -> Option<(Token, Symbol)> {
-        if path.arguments.is_empty() {
-            None
-        } else {
-            let property = GenericInstanceProperty {
-                base: base.id,
-                arguments: path.arguments.clone(),
-            };
-            let kind = SymbolKind::GenericInstance(property);
-            let token = &path.base;
-            let token = Token::new(
-                &path.mangled().to_string(),
-                token.line,
-                token.column,
-                token.length,
-                token.pos,
-                token.source,
-            );
-            let symbol = Symbol::new(&token, kind, &base.namespace, false, DocComment::default());
-            Some((token, symbol))
-        }
-    }
 }
 
 impl<'a> Handler for CreateReference<'a> {

crates/analyzer/src/handlers/create_symbol_table.rs

@@ -42,6 +42,7 @@ pub struct CreateSymbolTable<'a> {
     connect_targets: Vec<Vec<StrId>>,
     connects: HashMap<Token, Vec<Vec<StrId>>>,
     generic_parameters: Vec<SymbolId>,
+    needs_default_generic_argument: bool,
     generic_references: Vec<GenericSymbolPath>,
     default_clock_candidates: Vec<SymbolId>,
     defualt_reset_candidates: Vec<SymbolId>,
@@ -175,18 +176,6 @@ impl<'a> CreateSymbolTable<'a> {
             _ => false,
         }
     }
-
-    fn push_generic_parameter_item(
-        &mut self,
-        item: &WithGenericParameterItem,
-        default_value: Option<GenericSymbolPath>,
-    ) {
-        let property = GenericParameterProperty { default_value };
-        let kind = SymbolKind::GenericParameter(property);
-        if let Some(id) = self.insert_symbol(&item.identifier.identifier_token.token, kind, false) {
-            self.generic_parameters.push(id);
-        }
-    }
 }
 
 impl<'a> Handler for CreateSymbolTable<'a> {
@@ -622,11 +611,22 @@ impl<'a> VerylGrammarTrait for CreateSymbolTable<'a> {
 
     fn with_generic_parameter_list(
         &mut self,
-        arg: &WithGenericParameterList,
+        _arg: &WithGenericParameterList,
+    ) -> Result<(), ParolError> {
+        if let HandlerPoint::Before = self.point {
+            self.needs_default_generic_argument = false;
+        }
+        Ok(())
+    }
+
+    fn with_generic_parameter_item(
+        &mut self,
+        arg: &WithGenericParameterItem,
     ) -> Result<(), ParolError> {
         if let HandlerPoint::Before = self.point {
             let default_value: Option<GenericSymbolPath> =
-                if let Some(x) = &arg.with_generic_parameter_list_opt {
+                if let Some(ref x) = arg.with_generic_parameter_item_opt {
+                    self.needs_default_generic_argument = true;
                     match &*x.with_generic_argument_item {
                         WithGenericArgumentItem::ScopedIdentifier(x) => {
                             Some(x.scoped_identifier.as_ref().into())
@@ -637,40 +637,20 @@ impl<'a> VerylGrammarTrait for CreateSymbolTable<'a> {
                     None
                 };
 
-            if arg.with_generic_parameter_list_list.is_empty() {
-                self.push_generic_parameter_item(
-                    &arg.with_generic_parameter_item,
-                    default_value.clone(),
-                );
-            } else {
-                self.push_generic_parameter_item(&arg.with_generic_parameter_item, None);
-            }
-
-            for i in 0..arg.with_generic_parameter_list_list.len() {
-                if (i + 1) == arg.with_generic_parameter_list_list.len() {
-                    self.push_generic_parameter_item(
-                        &arg.with_generic_parameter_list_list[i].with_generic_parameter_item,
-                        default_value.clone(),
-                    );
-                } else {
-                    self.push_generic_parameter_item(
-                        &arg.with_generic_parameter_list_list[i].with_generic_parameter_item,
-                        None,
-                    );
-                }
-            }
-
-            if let Some(x) = &arg.with_generic_parameter_list_opt {
-                for x in &x.with_generic_parameter_list_opt_list {
-                    let default_value: Option<GenericSymbolPath> =
-                        match &*x.with_generic_argument_item {
-                            WithGenericArgumentItem::ScopedIdentifier(x) => {
-                                Some(x.scoped_identifier.as_ref().into())
-                            }
-                            WithGenericArgumentItem::Number(x) => Some(x.number.as_ref().into()),
-                        };
-                    self.push_generic_parameter_item(&x.with_generic_parameter_item, default_value);
+            if !self.needs_default_generic_argument || default_value.is_some() {
+                let property = GenericParameterProperty { default_value };
+                let kind = SymbolKind::GenericParameter(property);
+                if let Some(id) =
+                    self.insert_symbol(&arg.identifier.identifier_token.token, kind, false)
+                {
+                    self.generic_parameters.push(id);
                 }
+            } else {
+                self.errors.push(AnalyzerError::missing_default_argument(
+                    &arg.identifier.identifier_token.token.to_string(),
+                    self.text,
+                    &arg.identifier.as_ref().into(),
+                ));
             }
         }
         Ok(())

crates/analyzer/src/namespace.rs

@@ -47,20 +47,6 @@ impl Namespace {
         }
     }
 
-    pub fn is_subset(&self, x: &Namespace) -> bool {
-        if self.paths.len() < x.paths.len() {
-            return false;
-        } else {
-            for (i, x) in x.paths.iter().enumerate() {
-                if self.paths[i] != *x {
-                    return false;
-                }
-            }
-        }
-
-        true
-    }
-
     pub fn replace(&self, table: &HashMap<StrId, StrId>) -> Self {
         let mut paths = Vec::new();
         for x in &self.paths {

crates/analyzer/src/symbol.rs

@@ -183,50 +183,50 @@ impl Symbol {
     }
 
     pub fn generic_parameters(&self) -> Vec<(StrId, Option<GenericSymbolPath>)> {
+        fn get_generic_parameter(id: SymbolId) -> (StrId, Option<GenericSymbolPath>) {
+            let symbol = symbol_table::get(id).unwrap();
+            if let SymbolKind::GenericParameter(x) = symbol.kind {
+                (symbol.token.text, x.default_value)
+            } else {
+                unreachable!()
+            }
+        }
+
         match &self.kind {
             SymbolKind::Function(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             SymbolKind::Module(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             SymbolKind::Interface(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             SymbolKind::Package(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             SymbolKind::Struct(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             SymbolKind::Union(x) => x
                 .generic_parameters
                 .iter()
-                .map(|x| self.get_generic_parameter(*x))
+                .map(|x| get_generic_parameter(*x))
                 .collect(),
             _ => Vec::new(),
         }
     }
 
-    fn get_generic_parameter(&self, id: SymbolId) -> (StrId, Option<GenericSymbolPath>) {
-        let symbol = symbol_table::get(id).unwrap();
-        if let SymbolKind::GenericParameter(x) = symbol.kind {
-            (symbol.token.text, x.default_value)
-        } else {
-            unreachable!()
-        }
-    }
-
     pub fn generic_references(&self) -> Vec<GenericSymbolPath> {
         match &self.kind {
             SymbolKind::Function(x) => x.generic_references.clone(),

crates/analyzer/src/symbol_path.rs

@@ -1,6 +1,6 @@
 use crate::namespace::Namespace;
 use crate::namespace_table;
-use crate::symbol::SymbolKind;
+use crate::symbol::{DocComment, GenericInstanceProperty, Symbol, SymbolKind};
 use crate::symbol_table;
 use std::cmp::Ordering;
 use std::collections::HashMap;
@@ -213,6 +213,29 @@ impl GenericSymbol {
             resource_table::insert_str(&text)
         }
     }
+
+    pub fn get_generic_instance(&self, base: &Symbol) -> Option<(Token, Symbol)> {
+        if self.arguments.is_empty() {
+            None
+        } else {
+            let property = GenericInstanceProperty {
+                base: base.id,
+                arguments: self.arguments.clone(),
+            };
+            let kind = SymbolKind::GenericInstance(property);
+            let token = &self.base;
+            let token = Token::new(
+                &self.mangled().to_string(),
+                token.line,
+                token.column,
+                token.length,
+                token.pos,
+                token.source,
+            );
+            let symbol = Symbol::new(&token, kind, &base.namespace, false, DocComment::default());
+            Some((token, symbol))
+        }
+    }
 }
 
 impl GenericSymbolPath {

crates/analyzer/src/tests.rs

@@ -592,6 +592,60 @@ fn mismatch_function_arity() {
     ));
 }
 
+#[test]
+fn missing_default_generic_argument() {
+    let code = r#"
+    module ModuleA {
+        function FuncA::<A> () -> logic<A> {}
+        let _a: logic = FuncA::<1>();
+
+        function FuncB::<A, B, C> () -> logic<A + B + C> {}
+        let _b: logic = FuncB::<1, 2, 3>();
+
+        function FuncC::<A = 1> () -> logic<A> {}
+        let _c: logic = FuncC::<>();
+
+        function FuncD::<A = 1, B = 2, C = 3> () -> logic<A + B + C> {}
+        let _d: logic = FuncD::<>();
+
+        function FuncE::<A, B = 2, C = 3> () -> logic<A + B + C> {}
+        let _e: logic = FuncE::<1>();
+
+        function FuncF::<A, B, C = 3> () -> logic<A + B + C> {}
+        let _f: logic = FuncF::<1, 2>();
+    }
+    "#;
+
+    let errors = analyze(code);
+    assert!(errors.is_empty());
+
+    let code = r#"
+        module ModuleB {
+            function FuncA::<A = 1, B, C = 3> () -> logic<A + B + C> {}
+            let _a: logic = FuncA::<1, 2, 3> ();
+        }
+    "#;
+
+    let errors = analyze(code);
+    assert!(matches!(
+        errors[0],
+        AnalyzerError::MissingDefaultArgument { .. }
+    ));
+
+    let code = r#"
+        module ModuleC {
+            function FuncA::<A = 1, B = 2, Ccd> () -> logic<A + B + C> {}
+            let _a: logic = FuncA::<1, 2, 3>();
+        }
+    "#;
+
+    let errors = analyze(code);
+    assert!(matches!(
+        errors[0],
+        AnalyzerError::MissingDefaultArgument { .. }
+    ));
+}
+
 #[test]
 fn mismatch_generics_arity() {
     let code = r#"