PR #331: Rearrange spacing rules for ternary expression

Fixes #248. GitHub PR #331 Copybara import of the project: - faeb8a9 Rearrange spacing rules for ternary expression by Rafal Kapuscik <[email protected]> Closes #331 PiperOrigin-RevId: 317729847
chipsalliance · Jun 23, 2020 · c9d4d5e · c9d4d5e
1 parent c07a940
commit c9d4d5e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 10 deletions.
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -667,6 +667,10 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
         "  parameter  int   ternary=(a)?(b):(c);",
         "parameter int ternary = (a) ? (b) : (c);\n",
     },
+    {
+        "  parameter  int   ternary={a}?{b}:{c};",
+        "parameter int ternary = {a} ? {b} : {c};\n",
+    },
     // streaming operators
     {
         "   parameter  int  b={ >>   { a } } ;",
@@ -3183,6 +3187,11 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
      "    ++m;\n"
      "  end\n"
      "endfunction\n"},
+    {// spaces in ternary expression
+     "function f; return {a}? {b} :{ c };endfunction",
+     "function f;\n"
+     "  return {a} ? {b} : {c};\n"
+     "endfunction\n"},
     {"task t;endtask",
      "task t;\n"
      "endtask\n"},

diff --git a/verilog/formatting/token_annotator.cc b/verilog/formatting/token_annotator.cc
@@ -289,6 +289,17 @@ static WithReason<int> SpacesRequiredBetween(
     }
   }
 
+  if (left.TokenEnum() == ':') {
+    // Spacing in ranges
+    if (InRangeLikeContext(right_context)) {
+      // Take advantage here that the left token was already annotated (above)
+      return {left.before.spaces_required,
+              "Symmetrize spaces before and after ':' in bit slice"};
+    }
+    // Most contexts want a space after ':'.
+    return {1, "Default to 1 space after ':'"};
+  }
+
   if (left.TokenEnum() == '}') {
     // e.g. typedef struct { ... } foo_t;
     return {1, "Space after '}' in most other cases."};
@@ -403,16 +414,6 @@ static WithReason<int> SpacesRequiredBetween(
 
     // For now, if case is not explicitly handled, preserve existing space.
   }
-  if (left.TokenEnum() == ':') {
-    // Spacing in ranges
-    if (InRangeLikeContext(right_context)) {
-      // Take advantage here that the left token was already annotated (above)
-      return {left.before.spaces_required,
-              "Symmetrize spaces before and after ':' in bit slice"};
-    }
-    // Most contexts want a space after ':'.
-    return {1, "Default to 1 space after ':'"};
-  }
 
   // "if (...)", "for (...) instead of "if(...)", "for(...)",
   // "case ...", "return ..."

diff --git a/verilog/formatting/token_annotator_test.cc b/verilog/formatting/token_annotator_test.cc
@@ -2829,6 +2829,22 @@ TEST(TokenAnnotatorTest, AnnotateFormattingWithContextTest) {
           {NodeEnum::kTernaryExpression},
           {1, SpacingOptions::Undecided},
       },
+      {
+          // a ? {b} : {c} (ternary expression)
+          DefaultStyle,
+          {'}', "}"},
+          {':', ":"},
+          {NodeEnum::kTernaryExpression},
+          {1, SpacingOptions::Undecided},
+      },
+      {
+          // a ? {b} : {c} (ternary expression)
+          DefaultStyle,
+          {':', ":"},
+          {'{', "{"},
+          {NodeEnum::kTernaryExpression},
+          {1, SpacingOptions::Undecided},
+      },
 
       // ':' on the left, anything else on the right
       {