Report an error for select expressions after a range select

MikePopoloski · Nov 25, 2023 · a31e602 · a31e602
1 parent e9a9243
commit a31e602
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 6 deletions.
diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt
@@ -673,6 +673,7 @@ error SpecifyPathBadReference "cannot refer to '{}' in state-dependent path cond
 error PathPulseInExpr "PATHPULSE$ specparams cannot be used in expressions"
 error InterfacePortInvalidExpression "invalid expression for interface port connection"
 error AssignmentPatternLValueDynamic "lvalue assignment patterns cannot be assigned from dynamic arrays"
+error SelectAfterRangeSelect "cannot chain select expressions after a range select"
 warning ignored-slice IgnoredSlice "slice size ignored for left-to-right streaming operator"
 warning unsized-concat UnsizedInConcat "unsized integer in concat; using a width of {}"
 warning width-expand WidthExpand "implicit conversion expands from {} to {} bits"

diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp
@@ -1153,6 +1153,11 @@ Expression& Expression::bindSelector(Compilation& compilation, Expression& value
         return badExpr(compilation, nullptr);
     }
 
+    if (value.kind == ExpressionKind::RangeSelect) {
+        context.addDiag(diag::SelectAfterRangeSelect, syntax.sourceRange()) << value.sourceRange;
+        return badExpr(compilation, nullptr);
+    }
+
     // The full source range of the expression includes the value and the selector syntax.
     SourceRange fullRange = {value.sourceRange.start(), syntax.sourceRange().end()};
 

diff --git a/source/ast/Lookup.cpp b/source/ast/Lookup.cpp
@@ -1205,6 +1205,7 @@ const Symbol* Lookup::selectChild(const Symbol& initialSymbol,
                                   std::span<const ElementSelectSyntax* const> selectors,
                                   const ASTContext& context, LookupResult& result) {
     const Symbol* symbol = &initialSymbol;
+    const SyntaxNode* prevRangeSelect = nullptr;
     for (const ElementSelectSyntax* syntax : selectors) {
         if (symbol->kind != SymbolKind::InstanceArray &&
             symbol->kind != SymbolKind::GenerateBlockArray) {
@@ -1226,6 +1227,12 @@ const Symbol* Lookup::selectChild(const Symbol& initialSymbol,
         if (!syntax->selector)
             return selectorError();
 
+        if (prevRangeSelect) {
+            result.addDiag(*context.scope, diag::SelectAfterRangeSelect, syntax->sourceRange())
+                << prevRangeSelect->sourceRange();
+            return nullptr;
+        }
+
         switch (syntax->selector->kind) {
             case SyntaxKind::BitSelect:
                 symbol = selectSingleChild(*symbol, syntax->selector->as<BitSelectSyntax>(),
@@ -1236,6 +1243,7 @@ const Symbol* Lookup::selectChild(const Symbol& initialSymbol,
             case SyntaxKind::SimpleRangeSelect:
             case SyntaxKind::AscendingRangeSelect:
             case SyntaxKind::DescendingRangeSelect:
+                prevRangeSelect = syntax;
                 if (symbol->kind != SymbolKind::InstanceArray)
                     return selectorError();
 

diff --git a/tests/unittests/ast/ExpressionTests.cpp b/tests/unittests/ast/ExpressionTests.cpp
@@ -2799,7 +2799,7 @@ module m;
 
     function automatic int f1;
         int a = -1;
-        int b = y[a+:3][0];
+        int b[] = y[a+:3];
         w[0:1] = {1,1};
     endfunction
 
@@ -3143,3 +3143,23 @@ endmodule
     CHECK(diags[0].code == diag::ExpressionNotAssignable);
     CHECK(diags[1].code == diag::AssignmentPatternLValueDynamic);
 }
+
+TEST_CASE("Invalid select after range select") {
+    auto tree = SyntaxTree::fromText(R"(
+module m;
+    function int foo; return 1; endfunction
+
+    int i;
+    int j = i[3:0][0];
+    int k = foo()[3:0][0];
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 2);
+    CHECK(diags[0].code == diag::SelectAfterRangeSelect);
+    CHECK(diags[1].code == diag::SelectAfterRangeSelect);
+}
diff --git a/tests/unittests/ast/LookupTests.cpp b/tests/unittests/ast/LookupTests.cpp
@@ -529,13 +529,14 @@ module m;
 
     I arrayInv [-4:a] ();
     always_comb arrayInv[-4:-1][0].foo = 1;
-    always_comb array[-4:a][0].foo = 1;
+    always_comb array[-4:-1][0].foo = 1;
+    always_comb array[-4:a].foo = 1;
 
-    always_comb array[-1:-4][0].foo = 1;
-    always_comb array[-1+:-4][0].foo = 1;
-    always_comb array[-1+:4][0].foo = 1;
+    always_comb array[-1:-4].foo = 1;
+    always_comb array[-1+:-4].foo = 1;
+    always_comb array[-1+:4].foo = 1;
 
-    always_comb array[-2147483647-:3][0].foo = 1;
+    always_comb array[-2147483647-:3].foo = 1;
 endmodule
 )");
 
@@ -548,6 +549,7 @@ endmodule
     CHECK((it++)->code == diag::InvalidScopeIndexExpression);
     CHECK((it++)->code == diag::ScopeNotIndexable);
     CHECK((it++)->code == diag::UndeclaredIdentifier);
+    CHECK((it++)->code == diag::SelectAfterRangeSelect);
     CHECK((it++)->code == diag::UndeclaredIdentifier);
     CHECK((it++)->code == diag::InstanceArrayEndianMismatch);
     CHECK((it++)->code == diag::ValueMustBePositive);