Allow 0 spaces around binary operators inside more [] context.

Normally, the formatter pads 1 space around binary operators, but style guidance allows 0 in some contexts for compactness. We limit to 1 space max, and symmetrize the right-side spacing to follow the left-side spacing. You should see formatted results with "[a + b]" and "[a+b]". PiperOrigin-RevId: 329424493
chipsalliance · Sep 1, 2020 · 7fbda68 · 7fbda68
1 parent 287da29
commit 7fbda68
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 6 deletions.
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -582,7 +582,23 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
     },
     {
         "  parameter  int   foo=bar [ a+b ] ;",  // binary inside index expr
-        "parameter int foo = bar[a + b];\n",
+        "parameter int foo = bar[a+b];\n",       // allowed to be 0 spaces
+                                                 // (preserved)
+    },
+    {
+        "  parameter  int   foo=bar [ a+ b ] ;",  // binary inside index expr
+        "parameter int foo = bar[a+b];\n",        // allowed to be 0 spaces
+                                                  // (symmetrized)
+    },
+    {
+        "  parameter  int   foo=bar [ a +b ] ;",  // binary inside index expr
+        "parameter int foo = bar[a + b];\n",      // allowed to be 1 space
+                                                  // (symmetrized)
+    },
+    {
+        "  parameter  int   foo=bar [ a  +b ] ;",  // binary inside index expr
+        "parameter int foo = bar[a + b];\n",       // limited to 1 space (and
+                                                   // symmetrized)
     },
     {
         // with line continuations
@@ -657,7 +673,7 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
     {"  parameter int a={b^{c^{d^e}}};",
      "parameter int a = {b ^ {c ^ {d ^ e}}};\n"},
     {"  parameter int a={b^{c[d^e]}};",
-     "parameter int a = {b ^ {c[d ^ e]}};\n"},
+     "parameter int a = {b ^ {c[d^e]}};\n"},  // allow 0 spaces inside "[d^e]"
     {"  parameter int a={(b^c),(d^^e)};",
      "parameter int a = {(b ^ c), (d ^ ^e)};\n"},
 
@@ -1875,6 +1891,14 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
         "  if (foo);\n"
         "endmodule\n",
     },
+    {
+        "module conditional_generate;\n"
+        "if(foo[a*b+c])  ; \t"  // null action
+        "endmodule\n",
+        "module conditional_generate;\n"
+        "  if (foo[a*b+c]);\n"  // allow compact expressions inside []
+        "endmodule\n",
+    },
     {
         "module conditional_generate;\n"
         "if(foo)begin\n"

diff --git a/verilog/formatting/token_annotator.cc b/verilog/formatting/token_annotator.cc
@@ -233,6 +233,21 @@ static WithReason<int> SpacesRequiredBetween(
   // Consider assignment operators in the same class as binary operators.
   if (left.format_token_enum == FormatTokenType::binary_operator ||
       right.format_token_enum == FormatTokenType::binary_operator) {
+    // Inside [], allows 0 or 1 spaces, and symmetrize.
+    // TODO(fangism): make this behavior configurable
+    if (right.format_token_enum == FormatTokenType::binary_operator &&
+        InRangeLikeContext(right_context)) {
+      int spaces = right.OriginalLeadingSpaces().length();
+      if (spaces > 1) {
+        spaces = 1;
+      }
+      return {spaces, "Limit <= 1 space before binary operator inside []."};
+    }
+    if (left.format_token_enum == FormatTokenType::binary_operator &&
+        InRangeLikeContext(left_context)) {
+      return {left.before.spaces_required,
+              "Symmetrize spaces before and after binary operator inside []."};
+    }
     return {1, "Space around binary and assignment operators"};
   }