From a63a0aa0ccdc434144abba07a1d13056df77955e Mon Sep 17 00:00:00 2001 From: Nathaniel Bleier Date: Tue, 4 Jun 2024 09:06:08 -0500 Subject: [PATCH 1/4] Fixes #747 - Function identifier reffered as factor - and related bugs --- crates/analyzer/src/analyzer_error.rs | 25 +++++++++++++++++++++++++ crates/analyzer/src/handlers.rs | 6 ++++++ 2 files changed, 31 insertions(+) diff --git a/crates/analyzer/src/analyzer_error.rs b/crates/analyzer/src/analyzer_error.rs index a7e56de7..bd5c27cb 100644 --- a/crates/analyzer/src/analyzer_error.rs +++ b/crates/analyzer/src/analyzer_error.rs @@ -104,6 +104,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), @@ -812,6 +828,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, diff --git a/crates/analyzer/src/handlers.rs b/crates/analyzer/src/handlers.rs index 235d88f7..d64b73d0 100644 --- a/crates/analyzer/src/handlers.rs +++ b/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 } } From 3c340b47b7616c83e6f308b8551954ace9546e3f Mon Sep 17 00:00:00 2001 From: Nathaniel Bleier Date: Tue, 4 Jun 2024 09:06:59 -0500 Subject: [PATCH 2/4] need to add new file --- .../analyzer/src/handlers/check_expression.rs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 crates/analyzer/src/handlers/check_expression.rs diff --git a/crates/analyzer/src/handlers/check_expression.rs b/crates/analyzer/src/handlers/check_expression.rs new file mode 100644 index 00000000..30a1493d --- /dev/null +++ b/crates/analyzer/src/handlers/check_expression.rs @@ -0,0 +1,76 @@ +use crate::analyzer_error::AnalyzerError; +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, + 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 { + crate::symbol::SymbolKind::Function(_) + | crate::symbol::SymbolKind::ModportFunctionMember(_) + | crate::symbol::SymbolKind::SystemFunction => { + if x.factor_opt.is_none() { + self.errors.push(AnalyzerError::invalid_factor( + &identifier, + &rr.found.kind.to_kind_name(), + self.text, + &token, + )); + } + } + crate::symbol::SymbolKind::Module(_) + | crate::symbol::SymbolKind::Interface(_) + | crate::symbol::SymbolKind::Instance(_) + | crate::symbol::SymbolKind::Block + | crate::symbol::SymbolKind::Package(_) + | crate::symbol::SymbolKind::TypeDef(_) + | crate::symbol::SymbolKind::Enum(_) + | crate::symbol::SymbolKind::Modport(_) + | crate::symbol::SymbolKind::ModportVariableMember(_) + | crate::symbol::SymbolKind::Namespace + | crate::symbol::SymbolKind::GenericInstance(_) => { + self.errors.push(AnalyzerError::invalid_factor( + &identifier, + &rr.found.kind.to_kind_name(), + self.text, + &token, + )); + } + _ => {} + } + } + } + } + Ok(()) + } +} From 89219204a26163141be64f080b094424b3bb424d Mon Sep 17 00:00:00 2001 From: Nathaniel Bleier Date: Tue, 4 Jun 2024 09:10:55 -0500 Subject: [PATCH 3/4] added unit test for new error --- crates/analyzer/src/tests.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/analyzer/src/tests.rs b/crates/analyzer/src/tests.rs index 3c3fce0e..13a36114 100644 --- a/crates/analyzer/src/tests.rs +++ b/crates/analyzer/src/tests.rs @@ -1433,3 +1433,22 @@ fn reserved_identifier() { AnalyzerError::ReservedIdentifier { .. } )); } + +#[test] +fn invalid_factor_kind() { + let code = r#" + module ModuleA { + 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 { .. })); +} From a5022ac174bac4cb813db694b3c58a423656637e Mon Sep 17 00:00:00 2001 From: Nathaniel Bleier Date: Tue, 4 Jun 2024 09:49:55 -0500 Subject: [PATCH 4/4] Fixes #746 - Function call for non-function identifier --- crates/analyzer/src/analyzer_error.rs | 30 +++++++++++ .../analyzer/src/handlers/check_expression.rs | 52 ++++++++++++++----- crates/analyzer/src/tests.rs | 20 +++++++ 3 files changed, 88 insertions(+), 14 deletions(-) diff --git a/crates/analyzer/src/analyzer_error.rs b/crates/analyzer/src/analyzer_error.rs index bd5c27cb..a8cdc30a 100644 --- a/crates/analyzer/src/analyzer_error.rs +++ b/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), @@ -760,6 +776,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, diff --git a/crates/analyzer/src/handlers/check_expression.rs b/crates/analyzer/src/handlers/check_expression.rs index 30a1493d..b1748a03 100644 --- a/crates/analyzer/src/handlers/check_expression.rs +++ b/crates/analyzer/src/handlers/check_expression.rs @@ -1,4 +1,5 @@ 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; @@ -36,9 +37,9 @@ impl<'a> VerylGrammarTrait for CheckExpression<'a> { let identifier = rr.found.token.to_string(); let token: TokenRange = x.expression_identifier.as_ref().into(); match rr.found.kind { - crate::symbol::SymbolKind::Function(_) - | crate::symbol::SymbolKind::ModportFunctionMember(_) - | crate::symbol::SymbolKind::SystemFunction => { + SymbolKind::Function(_) + | SymbolKind::ModportFunctionMember(_) + | SymbolKind::SystemFunction => { if x.factor_opt.is_none() { self.errors.push(AnalyzerError::invalid_factor( &identifier, @@ -48,17 +49,17 @@ impl<'a> VerylGrammarTrait for CheckExpression<'a> { )); } } - crate::symbol::SymbolKind::Module(_) - | crate::symbol::SymbolKind::Interface(_) - | crate::symbol::SymbolKind::Instance(_) - | crate::symbol::SymbolKind::Block - | crate::symbol::SymbolKind::Package(_) - | crate::symbol::SymbolKind::TypeDef(_) - | crate::symbol::SymbolKind::Enum(_) - | crate::symbol::SymbolKind::Modport(_) - | crate::symbol::SymbolKind::ModportVariableMember(_) - | crate::symbol::SymbolKind::Namespace - | crate::symbol::SymbolKind::GenericInstance(_) => { + 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(), @@ -69,6 +70,29 @@ impl<'a> VerylGrammarTrait for CheckExpression<'a> { _ => {} } } + + 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(()) diff --git a/crates/analyzer/src/tests.rs b/crates/analyzer/src/tests.rs index 13a36114..22687472 100644 --- a/crates/analyzer/src/tests.rs +++ b/crates/analyzer/src/tests.rs @@ -1452,3 +1452,23 @@ fn invalid_factor_kind() { 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::CallNonFunction { .. })); +}