Merge pull request #753 from nblei/function_identifier_as_factor_fix

Function identifier as factor fix

crates/analyzer/src/analyzer_error.rs

@@ -4,6 +4,22 @@ use veryl_parser::veryl_token::TokenRange;
 
 #[derive(Error, Diagnostic, Debug)]
 pub enum AnalyzerError {
+    #[diagnostic(
+        severity(Error),
+        code(call_non_function),
+        help("remove call to non-function symbol"),
+        url("")
+    )]
+    #[error("Calling non-function symbol \"{identifier}\" which has kind \"{kind}\"")]
+    CallNonFunction {
+        identifier: String,
+        kind: String,
+        #[source_code]
+        input: NamedSource,
+        #[label("Error location")]
+        error_location: SourceSpan,
+    },
+
     #[diagnostic(
         severity(Error),
         code(cyclice_type_dependency),
@@ -104,6 +120,22 @@ pub enum AnalyzerError {
         error_location: SourceSpan,
     },
 
+    #[diagnostic(
+        severity(Error),
+        code(invalid_factor),
+        help("remove {kind} from expression"),
+        url("")
+    )]
+    #[error("{identifier} of kind \"{kind}\" cannot be used as a factor in an expression")]
+    InvalidFactor {
+        identifier: String,
+        kind: String,
+        #[source_code]
+        input: NamedSource,
+        #[label("Error location")]
+        error_location: SourceSpan,
+    },
+
     #[diagnostic(
         severity(Warning),
         code(invalid_identifier),
@@ -759,6 +791,20 @@ impl AnalyzerError {
         NamedSource::new(token.beg.source.to_string(), source.to_string())
     }
 
+    pub fn call_non_function(
+        identifier: &str,
+        kind: &str,
+        source: &str,
+        token: &TokenRange,
+    ) -> Self {
+        AnalyzerError::CallNonFunction {
+            identifier: identifier.into(),
+            kind: kind.into(),
+            input: AnalyzerError::named_source(source, token),
+            error_location: token.into(),
+        }
+    }
+
     pub fn cyclic_type_dependency(
         source: &str,
         start: &str,
@@ -827,6 +873,15 @@ impl AnalyzerError {
         }
     }
 
+    pub fn invalid_factor(identifier: &str, kind: &str, source: &str, token: &TokenRange) -> Self {
+        AnalyzerError::InvalidFactor {
+            identifier: identifier.to_string(),
+            kind: kind.to_string(),
+            input: AnalyzerError::named_source(source, token),
+            error_location: token.into(),
+        }
+    }
+
     pub fn invalid_identifier(
         identifier: &str,
         rule: &str,

crates/analyzer/src/handlers.rs

@@ -4,6 +4,7 @@ pub mod check_clock_reset;
 pub mod check_direction;
 pub mod check_embed_include;
 pub mod check_enum;
+pub mod check_expression;
 pub mod check_function;
 pub mod check_identifier;
 pub mod check_instance;
@@ -19,6 +20,7 @@ use check_clock_reset::*;
 use check_direction::*;
 use check_embed_include::*;
 use check_enum::*;
+use check_expression::*;
 use check_function::*;
 use check_identifier::*;
 use check_instance::*;
@@ -93,6 +95,7 @@ pub struct Pass2Handlers<'a> {
     check_clock_reset: CheckClockReset<'a>,
     create_reference: CreateReference<'a>,
     create_type_dag: CreateTypeDag<'a>,
+    check_expression: CheckExpression<'a>,
 }
 
 impl<'a> Pass2Handlers<'a> {
@@ -107,6 +110,7 @@ impl<'a> Pass2Handlers<'a> {
             check_clock_reset: CheckClockReset::new(text),
             create_reference: CreateReference::new(text),
             create_type_dag: CreateTypeDag::new(text),
+            check_expression: CheckExpression::new(text),
         }
     }
 
@@ -121,6 +125,7 @@ impl<'a> Pass2Handlers<'a> {
             &mut self.check_clock_reset as &mut dyn Handler,
             &mut self.create_reference as &mut dyn Handler,
             &mut self.create_type_dag as &mut dyn Handler,
+            &mut self.check_expression as &mut dyn Handler,
         ]
     }
 
@@ -135,6 +140,7 @@ impl<'a> Pass2Handlers<'a> {
         ret.append(&mut self.check_clock_reset.errors);
         ret.append(&mut self.create_reference.errors);
         ret.append(&mut self.create_type_dag.errors);
+        ret.append(&mut self.check_expression.errors);
         ret
     }
 }

crates/analyzer/src/handlers/check_expression.rs

@@ -0,0 +1,100 @@
+use crate::analyzer_error::AnalyzerError;
+use crate::symbol::SymbolKind;
+use crate::symbol_table;
+use veryl_parser::veryl_grammar_trait::*;
+use veryl_parser::veryl_token::TokenRange;
+use veryl_parser::veryl_walker::{Handler, HandlerPoint};
+use veryl_parser::ParolError;
+
+#[derive(Default)]
+pub struct CheckExpression<'a> {
+    pub errors: Vec<AnalyzerError>,
+    text: &'a str,
+    point: HandlerPoint,
+}
+
+impl<'a> CheckExpression<'a> {
+    pub fn new(text: &'a str) -> Self {
+        Self {
+            text,
+            ..Default::default()
+        }
+    }
+}
+
+impl<'a> Handler for CheckExpression<'a> {
+    fn set_point(&mut self, p: HandlerPoint) {
+        self.point = p;
+    }
+}
+
+impl<'a> VerylGrammarTrait for CheckExpression<'a> {
+    fn factor(&mut self, arg: &Factor) -> Result<(), ParolError> {
+        if let HandlerPoint::Before = self.point {
+            if let Factor::ExpressionIdentifierFactorOpt(x) = arg {
+                let expid = x.expression_identifier.as_ref();
+                if let Ok(rr) = symbol_table::resolve(expid) {
+                    let identifier = rr.found.token.to_string();
+                    let token: TokenRange = x.expression_identifier.as_ref().into();
+                    match rr.found.kind {
+                        SymbolKind::Function(_)
+                        | SymbolKind::ModportFunctionMember(_)
+                        | SymbolKind::SystemFunction => {
+                            if x.factor_opt.is_none() {
+                                self.errors.push(AnalyzerError::invalid_factor(
+                                    &identifier,
+                                    &rr.found.kind.to_kind_name(),
+                                    self.text,
+                                    &token,
+                                ));
+                            }
+                        }
+                        SymbolKind::Module(_)
+                        | SymbolKind::Interface(_)
+                        | SymbolKind::Instance(_)
+                        | SymbolKind::Block
+                        | SymbolKind::Package(_)
+                        | SymbolKind::TypeDef(_)
+                        | SymbolKind::Enum(_)
+                        | SymbolKind::Modport(_)
+                        | SymbolKind::ModportVariableMember(_)
+                        | SymbolKind::Namespace
+                        | SymbolKind::GenericInstance(_) => {
+                            self.errors.push(AnalyzerError::invalid_factor(
+                                &identifier,
+                                &rr.found.kind.to_kind_name(),
+                                self.text,
+                                &token,
+                            ));
+                        }
+                        _ => {}
+                    }
+                }
+
+                if x.factor_opt.is_some() {
+                    // Must be a function call
+                    let expid = x.expression_identifier.as_ref();
+                    if let Ok(rr) = symbol_table::resolve(expid) {
+                        match rr.found.kind {
+                            SymbolKind::Function(_)
+                            | SymbolKind::SystemVerilog
+                            | SymbolKind::ModportFunctionMember(..)
+                            | SymbolKind::SystemFunction => {}
+                            _ => {
+                                let identifier = rr.found.token.to_string();
+                                let token: TokenRange = x.expression_identifier.as_ref().into();
+                                self.errors.push(AnalyzerError::call_non_function(
+                                    &identifier,
+                                    &rr.found.kind.to_kind_name(),
+                                    self.text,
+                                    &token,
+                                ));
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        Ok(())
+    }
+}

crates/analyzer/src/tests.rs

@@ -1435,13 +1435,40 @@ fn reserved_identifier() {
 }
 
 #[test]
-fn sv_keyword_usage() {
+fn invalid_factor_kind() {
     let code = r#"
     module ModuleA {
-        var pure: logic;
-    }
-    "#;
+        function f (
+            a: input logic,
+        ) -> logic {
+            return a;
+        }
+
+        var a: logic;
+
+        assign a = f + 1;
+    }"#;
+
+    let errors = analyze(code);
+    assert!(matches!(errors[0], AnalyzerError::InvalidFactor { .. }));
+}
+
+#[test]
+fn call_non_function() {
+    let code = r#"
+    module ModuleA {
+        function f (
+            a: input logic,
+        ) -> logic {
+            return a;
+        }
+
+        var a: logic;
+        var b: logic;
+
+        assign a = b() + 1;
+    }"#;
 
     let errors = analyze(code);
-    assert!(matches!(errors[0], AnalyzerError::SvKeywordUsage { .. }));
+    assert!(matches!(errors[0], AnalyzerError::CallNonFunction { .. }));
 }