diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc index d43be0a63..c0c37dd19 100644 --- 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 index f7c37ad5c..cac9f871b 100644 --- a/verilog/formatting/token_annotator.cc +++ b/verilog/formatting/token_annotator.cc @@ -233,6 +233,21 @@ static WithReason 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"}; } diff --git a/verilog/formatting/token_annotator_test.cc b/verilog/formatting/token_annotator_test.cc index b12d85f53..164493eb1 100644 --- a/verilog/formatting/token_annotator_test.cc +++ b/verilog/formatting/token_annotator_test.cc @@ -4665,23 +4665,29 @@ TEST(TokenAnnotatorTest, AnnotateFormattingWithContextTest) { } } // NOLINT(readability/fn_size) -struct AnnotateBreakAroundCommentsTestCase { +struct OriginalSpacingSensitiveTestCase { FormatStyle style; + // TODO(fangism): group this into a TokenInfo. int left_token_enum; absl::string_view left_token_string; + // This spacing may influence token-annotation behavior. absl::string_view whitespace_between; + // TODO(fangism): group this into a TokenInfo. int right_token_enum; absl::string_view right_token_string; + InitializedSyntaxTreeContext left_context; InitializedSyntaxTreeContext right_context; + ExpectedInterTokenInfo expected_annotation; }; -TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { - const AnnotateBreakAroundCommentsTestCase kTestCases[] = { +// These tests are allowed to be sensitive to original inter-token spacing. +TEST(TokenAnnotatorTest, OriginalSpacingSensitiveTests) { + const OriginalSpacingSensitiveTestCase kTestCases[] = { {// No comments DefaultStyle, '=', // left token @@ -4690,6 +4696,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_DecNumber, // right token "0", {/* unspecified context */}, + {/* unspecified context */}, {1, SpacingOptions::Undecided}}, {// //comment1 // //comment2 @@ -4700,6 +4707,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "//comment2", {}, + {}, {2, SpacingOptions::MustWrap}}, {// 0 // comment DefaultStyle, @@ -4709,6 +4717,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// 0// comment DefaultStyle, @@ -4718,6 +4727,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// 0 \n // comment DefaultStyle, @@ -4727,6 +4737,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// // comment 1 \n // comment 2 DefaultStyle, @@ -4736,6 +4747,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustWrap}}, {// /* comment 1 */ \n // comment 2 DefaultStyle, @@ -4745,6 +4757,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// /* comment 1 */ // comment 2 DefaultStyle, @@ -4754,6 +4767,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// ; // comment 2 DefaultStyle, @@ -4763,6 +4777,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// ; \n // comment 2 DefaultStyle, @@ -4772,6 +4787,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// , // comment 2 DefaultStyle, @@ -4781,6 +4797,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// , \n // comment 2 DefaultStyle, @@ -4790,6 +4807,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// begin // comment 2 DefaultStyle, @@ -4799,6 +4817,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// begin \n // comment 2 DefaultStyle, @@ -4808,6 +4827,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// else // comment 2 DefaultStyle, @@ -4817,6 +4837,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// else \n // comment 2 DefaultStyle, @@ -4826,6 +4847,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// end // comment 2 DefaultStyle, @@ -4835,6 +4857,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// end \n // comment 2 DefaultStyle, @@ -4844,6 +4867,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// generate // comment 2 DefaultStyle, @@ -4853,6 +4877,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// generate \n // comment 2 DefaultStyle, @@ -4862,6 +4887,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, {// if // comment 2 DefaultStyle, @@ -4871,6 +4897,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}}, {// if \n\n // comment 2 DefaultStyle, @@ -4880,6 +4907,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment 2", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}}, { DefaultStyle, @@ -4889,6 +4917,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "//comment", {/* any context */}, + {/* any context */}, {0, SpacingOptions::MustWrap}, }, { @@ -4899,6 +4928,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/*comment*/", {/* any context */}, + {/* any context */}, {0, SpacingOptions::MustWrap}, }, { @@ -4909,6 +4939,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/*comment*/", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}, // could be append }, { @@ -4919,6 +4950,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/*comment*/", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}, }, { @@ -4929,6 +4961,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "//comment", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::MustAppend}, }, { @@ -4939,6 +4972,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "//comment", {/* unspecified context */}, + {/* unspecified context */}, {2, SpacingOptions::Undecided}, }, // Comments in UDP entries @@ -4951,6 +4985,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/* comment */", {NodeEnum::kUdpCombEntry}, + {NodeEnum::kUdpCombEntry}, {2, SpacingOptions::Undecided}, }, { @@ -4962,6 +4997,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { '0', "0", {NodeEnum::kUdpCombEntry}, + {NodeEnum::kUdpCombEntry}, {1, SpacingOptions::Undecided}, }, { @@ -4973,6 +5009,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {NodeEnum::kUdpCombEntry}, + {NodeEnum::kUdpCombEntry}, {2, SpacingOptions::MustAppend}, }, { @@ -4984,6 +5021,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/* comment */", {NodeEnum::kUdpSequenceEntry}, + {NodeEnum::kUdpSequenceEntry}, {2, SpacingOptions::Undecided}, }, { @@ -4995,6 +5033,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { '0', "0", {NodeEnum::kUdpSequenceEntry}, + {NodeEnum::kUdpSequenceEntry}, {1, SpacingOptions::Undecided}, }, { @@ -5006,6 +5045,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {NodeEnum::kUdpSequenceEntry}, + {NodeEnum::kUdpSequenceEntry}, {2, SpacingOptions::MustAppend}, }, { @@ -5017,6 +5057,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/* comment */", {NodeEnum::kUdpPortDeclaration}, + {NodeEnum::kUdpPortDeclaration}, {2, SpacingOptions::Undecided}, }, { @@ -5028,6 +5069,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::SymbolIdentifier, "i", {NodeEnum::kUdpPortDeclaration}, + {NodeEnum::kUdpPortDeclaration}, {1, SpacingOptions::Undecided}, }, { @@ -5039,6 +5081,7 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_COMMENT_BLOCK, "/* comment */", {NodeEnum::kUdpPortDeclaration}, + {NodeEnum::kUdpPortDeclaration}, {2, SpacingOptions::Undecided}, }, { @@ -5050,14 +5093,52 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { verilog_tokentype::TK_EOL_COMMENT, "// comment", {NodeEnum::kUdpPortDeclaration}, + {NodeEnum::kUdpPortDeclaration}, {2, SpacingOptions::MustAppend}, }, + + { + // [a+b] + DefaultStyle, + verilog_tokentype::SymbolIdentifier, + "a", + "", // no spaces originally + '+', + "+", + {NodeEnum::kSelectVariableDimension}, + {NodeEnum::kSelectVariableDimension}, + {0, SpacingOptions::Undecided}, + }, + { + // [a +b] + DefaultStyle, + verilog_tokentype::SymbolIdentifier, + "a", + " ", // 1 space originally + '+', + "+", + {NodeEnum::kSelectVariableDimension}, + {NodeEnum::kSelectVariableDimension}, + {1, SpacingOptions::Undecided}, + }, + { + // [a +b] + DefaultStyle, + verilog_tokentype::SymbolIdentifier, + "a", + " ", // 2 spaces originally + '+', + "+", + {NodeEnum::kSelectVariableDimension}, + {NodeEnum::kSelectVariableDimension}, + {1, SpacingOptions::Undecided}, // limit to 1 + }, }; int test_index = 0; for (const auto& test_case : kTestCases) { VLOG(1) << "test_index[" << test_index << "]:"; - verible::TokenInfoTestData test_data = { + const verible::TokenInfoTestData test_data = { {test_case.left_token_enum, test_case.left_token_string}, test_case.whitespace_between, {test_case.right_token_enum, test_case.right_token_string}}; @@ -5067,6 +5148,8 @@ TEST(TokenAnnotatorTest, AnnotateBreakAroundComments) { PreFormatToken left(&token_vector[0]); PreFormatToken right(&token_vector[1]); + // like verible::ConnectPreFormatTokensPreservedSpaceStarts(); + right.before.preserved_space_start = left.Text().end(); left.format_token_enum = GetFormatTokenType(verilog_tokentype(left.TokenEnum()));