Merge pull request #2851 from mvanhorn/fix/2543-for-bare-expression-error

fix(analyzer): report an error for a bare for-loop range

Author: dalance

crates/analyzer/src/analyzer_error.rs

@@ -707,6 +707,21 @@ pub enum AnalyzerError {
         token_source: TokenSource,
     },
 
+    #[diagnostic(
+        severity(Error),
+        code(invalid_for_range),
+        help("use a range with `..` or `..=`, e.g. `0..N`"),
+        url("https://doc.veryl-lang.org/book/07_appendix/02_semantic_error.html#{}", self.code().unwrap())
+    )]
+    #[error("for-loop range must use `..` or `..=`; a bare expression is not a valid range")]
+    InvalidForRange {
+        #[source_code]
+        input: MultiSources,
+        #[label("Error location")]
+        error_location: SourceSpan,
+        token_source: TokenSource,
+    },
+
     #[diagnostic(
         severity(Error),
         code(invalid_for_step),
@@ -1982,6 +1997,7 @@ impl AnalyzerError {
             AnalyzerError::InvalidEmbedIdentifier { input, .. } => input,
             AnalyzerError::InvalidEnumVariant { input, .. } => input,
             AnalyzerError::InvalidFactor { input, .. } => input,
+            AnalyzerError::InvalidForRange { input, .. } => input,
             AnalyzerError::InvalidForStep { input, .. } => input,
             AnalyzerError::InvalidIdentifier { input, .. } => input,
             AnalyzerError::InvalidImport { input, .. } => input,
@@ -2109,6 +2125,7 @@ impl AnalyzerError {
             AnalyzerError::InvalidRangeAssign { token_source, .. } => *token_source,
             AnalyzerError::NonConstantSelectWidth { token_source, .. } => *token_source,
             AnalyzerError::InvalidStatement { token_source, .. } => *token_source,
+            AnalyzerError::InvalidForRange { token_source, .. } => *token_source,
             AnalyzerError::InvalidForStep { token_source, .. } => *token_source,
             AnalyzerError::InvalidTbUsage { token_source, .. } => *token_source,
             AnalyzerError::MissingTbPort { token_source, .. } => *token_source,
@@ -2564,6 +2581,13 @@ impl AnalyzerError {
             token_source: token.source(),
         }
     }
+    pub fn invalid_for_range(token: &TokenRange) -> Self {
+        AnalyzerError::InvalidForRange {
+            input: source(token),
+            error_location: token.into(),
+            token_source: token.source(),
+        }
+    }
     pub fn invalid_for_step(cause: InvalidForStepKind, token: &TokenRange) -> Self {
         AnalyzerError::InvalidForStep {
             cause,

crates/analyzer/src/handlers/check_statement.rs

@@ -2,6 +2,7 @@ use crate::analyzer_error::AnalyzerError;
 use crate::attribute::Attribute;
 use crate::attribute_table;
 use veryl_parser::ParolError;
+use veryl_parser::token_range::TokenRange;
 use veryl_parser::veryl_grammar_trait::*;
 use veryl_parser::veryl_walker::{Handler, HandlerPoint};
 
@@ -116,14 +117,30 @@ impl VerylGrammarTrait for CheckStatement {
         Ok(())
     }
 
-    fn for_statement(&mut self, _arg: &ForStatement) -> Result<(), ParolError> {
+    fn for_statement(&mut self, arg: &ForStatement) -> Result<(), ParolError> {
         match self.point {
-            HandlerPoint::Before => self.statement_depth_in_loop += 1,
+            HandlerPoint::Before => {
+                self.statement_depth_in_loop += 1;
+                if arg.range.range_opt.is_none() {
+                    let token: TokenRange = arg.range.as_ref().into();
+                    self.errors.push(AnalyzerError::invalid_for_range(&token));
+                }
+            }
             HandlerPoint::After => self.statement_depth_in_loop -= 1,
         }
         Ok(())
     }
 
+    fn generate_for_declaration(&mut self, arg: &GenerateForDeclaration) -> Result<(), ParolError> {
+        if let HandlerPoint::Before = self.point
+            && arg.range.range_opt.is_none()
+        {
+            let token: TokenRange = arg.range.as_ref().into();
+            self.errors.push(AnalyzerError::invalid_for_range(&token));
+        }
+        Ok(())
+    }
+
     fn always_ff_declaration(&mut self, _arg: &AlwaysFfDeclaration) -> Result<(), ParolError> {
         match self.point {
             HandlerPoint::Before => {

crates/analyzer/src/tests.rs

@@ -1898,6 +1898,67 @@ fn invalid_statement() {
     assert!(matches!(errors[0], AnalyzerError::InvalidStatement { .. }));
 }
 
+#[test]
+fn invalid_for_range() {
+    let code = r#"
+    module ModuleA {
+        always_comb {
+            for i in 4 {
+            }
+        }
+    }
+    "#;
+
+    let errors = analyze(code);
+    assert!(
+        errors
+            .iter()
+            .any(|e| matches!(e, AnalyzerError::InvalidForRange { .. })),
+        "{errors:?}"
+    );
+
+    let code = r#"
+    module ModuleA {
+        for i in 4 :blk {
+        }
+    }
+    "#;
+
+    let errors = analyze(code);
+    assert!(
+        errors
+            .iter()
+            .any(|e| matches!(e, AnalyzerError::InvalidForRange { .. })),
+        "{errors:?}"
+    );
+
+    let code = r#"
+    module ModuleA (
+        o: output logic<32>,
+    ) {
+        always_comb {
+            var acc: logic<32>;
+            acc = 0;
+            for i in 0..4 {
+                acc += i;
+            }
+            for j in 0..=4 {
+                acc += j;
+            }
+            o = acc;
+        }
+    }
+    "#;
+
+    let errors = analyze(code);
+    assert!(
+        !errors
+            .iter()
+            .any(|e| matches!(e, AnalyzerError::InvalidForRange { .. })),
+        "{errors:?}"
+    );
+}
+
 #[test]
 fn invalid_modport_item() {
     let code = r#"

crates/tests/src/snapshots/invalid_for_range.snap

@@ -0,0 +1,15 @@
+---
+source: crates/tests/src/lib.rs
+expression: out
+---
+invalid_for_range (https://doc.veryl-lang.org/book/07_appendix/02_semantic_error.html#invalid_for_range)
+
+  × for-loop range must use `..` or `..=`; a bare expression is not a valid range
+   ╭─[../../testcases/error/invalid_for_range.veryl:5:18]
+ 4 │         a = 0;
+ 5 │         for i in 4 {
+   ·                  ┬
+   ·                  ╰── Error location
+ 6 │             a += i;
+   ╰────
+  help: use a range with `..` or `..=`, e.g. `0..N`

testcases/error/invalid_for_range.veryl

@@ -0,0 +1,9 @@
+module invalid_for_range {
+    var a: logic<32>;
+    always_comb {
+        a = 0;
+        for i in 4 {
+            a += i;
+        }
+    }
+}