formatter: Preserve operator spacing in ternary expressions inside subscript brackets `InRangeLikeContext()` previously matched all tokens nested inside subscript dimension nodes (`kDimensionScalar`), causing compact indexing rules to strip spaces around operators even within ternary condition expressions (`kConditionExpression`). This produced invalid syntax like `some_array[b>1'h0?1'h0 : c]`. Exclude `kConditionExpression` in `InRangeLikeContext()` so operators inside ternary expressions within subscript brackets retain normal spacing rules. Verified #2597 with new test case `FormatterEndToEndTest.TernaryInsideSubscriptExpression` Fixes #2597
diff --git a/.bazelrc b/.bazelrc index 301b564..dc2e33d 100644 --- a/.bazelrc +++ b/.bazelrc
@@ -1,6 +1,8 @@ # bazel < 7 needs explicit enabling of bzlmod dependencies. build --enable_bzlmod +test --test_output=errors + # Enable support for absl types like string_view in gtest. build --define="absl=1"
diff --git a/verible/verilog/formatting/formatter_module_test.cc b/verible/verilog/formatting/formatter_module_test.cc index f6e142b..c61da86 100644 --- a/verible/verilog/formatting/formatter_module_test.cc +++ b/verible/verilog/formatting/formatter_module_test.cc
@@ -3050,6 +3050,40 @@ RunFormatterTestCases40(kModuleFormatterTestCases); } +TEST(FormatterEndToEndTest, TernaryInsideSubscriptExpression_issue2597) { + static constexpr FormatterTestCase kTestCases[] = { + // Outside subscript + {"module foo ();\n" + "assign a = b > 1'h0 ? 1'h0 : c;\n" + "endmodule\n", + + "module foo ();\n" + " assign a = b > 1'h0 ? 1'h0 : c;\n" + "endmodule\n"}, + + // Inside subscript. + {"module foo ();\n" + "assign a = some_array[b > 1'h0 ? 1'h0 : c];\n" + "endmodule\n", + + "module foo ();\n" + " assign a = some_array[b > 1'h0 ? 1'h0 : c];\n" + "endmodule\n"}, + }; + + FormatStyle style; + style.indentation_spaces = 2; + for (const auto &test_case : kTestCases) { + VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>"; + std::ostringstream stream; + const auto status = + FormatVerilog(test_case.input, "<filename>", style, stream); + // Require these test cases to be valid. + EXPECT_OK(status) << status.message(); + EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input; + } +} + } // namespace } // namespace formatter } // namespace verilog
diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc index fc6d2f0..959364e 100644 --- a/verible/verilog/formatting/token-annotator.cc +++ b/verible/verilog/formatting/token-annotator.cc
@@ -117,7 +117,7 @@ return context.IsInsideFirst( {NodeEnum::kDimensionScalar, NodeEnum::kDimensionRange, NodeEnum::kDimensionSlice, NodeEnum::kCycleDelayRange}, - {}); + {NodeEnum::kConditionExpression}); // exclude } static bool IsAnySemicolon(const PreFormatToken &ftoken) {
diff --git a/verible/verilog/formatting/token-annotator_test.cc b/verible/verilog/formatting/token-annotator_test.cc index 2adf936..0bd6485 100644 --- a/verible/verilog/formatting/token-annotator_test.cc +++ b/verible/verilog/formatting/token-annotator_test.cc
@@ -5501,6 +5501,31 @@ {NodeEnum::kUnpackedDimensions}, {1, SpacingOptions::kPreserve}, }, + { + // [b > 1'h0 ? 1'h0 : c] : space around '>' in ternary inside + // subscript + DefaultStyle, + verilog_tokentype::SymbolIdentifier, + "b", + " ", // 1 space originally + '>', + ">", + {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression}, + {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression}, + {1, SpacingOptions::kUndecided}, + }, + { + // [b > 1 ? 1 : c] : space before '?' in ternary inside subscript + DefaultStyle, + verilog_tokentype::TK_DecNumber, + "1", + " ", // 1 space originally + '?', + "?", + {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression}, + {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression}, + {1, SpacingOptions::kUndecided}, + }, }; int test_index = 0; for (const auto &test_case : kTestCases) {