Merge pull request #1885 from antmicro/add-macro-in-struct-body-test

Added macro call in struct test without a semicolon.
chipsalliance · Apr 27, 2023 · d600c4a · d600c4a
2 parents ddd81e4 + 784f205
commit d600c4a
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 4 deletions.
diff --git a/verilog/analysis/verilog_equivalence.cc b/verilog/analysis/verilog_equivalence.cc
@@ -167,7 +167,11 @@ DiffStatus LexicallyEquivalent(
   DiffStatus diff_status = DiffStatus::kEquivalent;
   auto recursive_comparator = [&](const TokenSequence::const_iterator l,
                                   const TokenSequence::const_iterator r) {
-    if (l->token_enum() != r->token_enum()) {
+    if (l->token_enum() != r->token_enum() &&
+        !((l->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+           r->text() == ")") ||
+          (r->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+           l->text() == ")"))) {
       if (errstream != nullptr) {
         *errstream << "Mismatched token enums.  got: ";
         token_printer(*l, *errstream);
@@ -246,6 +250,14 @@ DiffStatus FormatEquivalent(absl::string_view left, absl::string_view right,
         return IsWhitespace(verilog_tokentype(t.token_enum()));
       },
       [=](const TokenInfo& l, const TokenInfo& r) {
+        // MacroCallCloseToEndLine should be considered equivalent to ')', as
+        // they are whitespace dependant
+        if (((r.token_enum() == verilog_tokentype::MacroCallCloseToEndLine) &&
+             (l.text() == ")")) ||
+            ((l.token_enum() == verilog_tokentype::MacroCallCloseToEndLine) &&
+             (r.text() == ")"))) {
+          return true;
+        }
         return l.EquivalentWithoutLocation(r);
       },
       errstream);

diff --git a/verilog/analysis/verilog_equivalence_test.cc b/verilog/analysis/verilog_equivalence_test.cc
@@ -238,6 +238,25 @@ TEST(FormatEquivalentTest, MismatchTokenType) {
       << errs.str();
 }
 
+TEST(FormatEquivalentTest, EquivalenceOfRightParen) {
+  const char* kTestCases[] = {
+      "{`FOO()\n}\n",
+      "{`FOO()}\n",
+      "{`FOO()()}\n",
+      "{`FOO()()\n}\n",
+  };
+  {
+    std::ostringstream errs;
+    ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kEquivalent,
+                               kTestCases[0], kTestCases[1], &errs);
+    // Test the other way around too.
+    ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kEquivalent,
+                               kTestCases[1], kTestCases[0], &errs);
+    ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kEquivalent,
+                               kTestCases[2], kTestCases[3], &errs);
+  }
+}
+
 TEST(FormatEquivalentTest, DiagnosticMismatch) {
   const char* kTestCases[] = {
       "module foo;\n",

diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -8271,6 +8271,21 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
      "    }\n"
      ") ();\n"
      "endmodule\n"},
+    // The same as the previous test case, but without a semicolon at the macro
+    // call
+    {"module foo\n"
+     "#(\n"
+     "  parameter type baz_t = struct packed { `BAZ() }\n"
+     ")\n"
+     "();\n"
+     "endmodule\n",
+     "module foo #(\n"
+     "    parameter type\n"
+     "        baz_t = struct packed {\n"
+     "      `BAZ()\n"
+     "    }\n"
+     ") ();\n"
+     "endmodule\n"},
     // Check that comments with too large starting column difference are not
     // aligned as continuation comments.
     // Check that starting comments are not linked with a comment in

diff --git a/verilog/formatting/token_annotator.cc b/verilog/formatting/token_annotator.cc
@@ -17,6 +17,7 @@
 #include <iterator>
 #include <vector>
 
+#include "absl/strings/match.h"
 #include "absl/strings/string_view.h"
 #include "common/formatting/format_token.h"
 #include "common/formatting/tree_annotator.h"
@@ -437,7 +438,9 @@ static WithReason<int> SpacesRequiredBetween(
     if (InRangeLikeContext(right_context)) {
       int spaces = right.OriginalLeadingSpaces().length();
       if (spaces > 1) {
-        spaces = 1;
+        // If ExcessSpaces returns 0 if there was a newline - prevents
+        // counting indentation as spaces
+        spaces = right.ExcessSpaces() ? 1 : 0;
       }
       return {spaces, "Limit spaces before ':' in bit slice to 0 or 1"};
     }

diff --git a/verilog/formatting/token_annotator_test.cc b/verilog/formatting/token_annotator_test.cc
@@ -5317,6 +5317,18 @@ TEST(TokenAnnotatorTest, OriginalSpacingSensitiveTests) {
           {NodeEnum::kDimensionRange},
           {0, SpacingOptions::kUndecided},
       },
+      {
+          DefaultStyle,
+          verilog_tokentype::SymbolIdentifier,
+          "a",
+          "\n    ",
+          ':',
+          ":",
+          {NodeEnum::kDimensionRange},
+          {NodeEnum::kDimensionRange},
+          // 0 spaces as this is an indentation, not spacing
+          {0, SpacingOptions::kUndecided},
+      },
       {
           DefaultStyle,
           '*',

diff --git a/verilog/parser/verilog.y b/verilog/parser/verilog.y
@@ -3685,11 +3685,13 @@ struct_union_member
     { $$ = MakeTaggedNode(N::kStructUnionMember, $1, $2, $3, $4, $5, $6); }
   | preprocessor_directive
     { $$ = std::move($1); }
-  | MacroCall
-    { $$ = std::move($1);}
   // since the semicolon is optional, it is better to have both cases covered
   | MacroCall ';'
     { $$ = ExtendNode($1, $2);}
+  | MacroCall
+    { $$ = std::move($1);}
+  | MacroCallId '(' macro_args_opt MacroCallCloseToEndLine
+    { $$ = MakeTaggedNode(N::kMacroCall, $1, MakeParenGroup($2, $3, $4)); }
   ;
 
 case_item

diff --git a/verilog/parser/verilog_parser_unittest.cc b/verilog/parser/verilog_parser_unittest.cc
@@ -3771,6 +3771,9 @@ static constexpr ParserTestCaseArray kStructTests = {
     "struct packed { int i; bit b; } foo;",
     "struct packed signed { int i; bit b; } foo;",
     "struct packed unsigned { int i; bit b; } foo;",
+    "struct { `BAZ() } foo;",
+    "struct { `BAZ(); } foo;",
+    "struct { `BAZ()\n } foo;",
 };
 
 static constexpr ParserTestCaseArray kEnumTests = {