Skip to content

Commit

Permalink
Keep `define macro together (must-append).
Browse files Browse the repository at this point in the history 
Until macro re-flowing is implemented, it is best to keep
* `define attached to the ID
* ID attached to the macro definition text (which may start with '\')

PiperOrigin-RevId: 293240982
  • Loading branch information
@fangism @hzeller
fangism authored and hzeller committed Feb 5, 2020
1 parent ca3fce4 commit 81f6540
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 21 deletions.
8 changes: 8 additions & 0 deletions verilog/formatting/formatter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
"`ifndef FOO\n"
"`define BAR\n\n"
"`endif\n"},
{"`define FOO \\\n"
" BAR\n",
"`define FOO \\\n" // TODO(b/72527558): right align '\'s to column limit
" BAR\n"},
{"`define FOOOOOOOOOOOOOOOO \\\n"
" BAAAAAAAAAAAAAAAAR BAAAAAAAAAAAAAZ;\n",
"`define FOOOOOOOOOOOOOOOO \\\n" // macro text starts at '\'
" BAAAAAAAAAAAAAAAAR BAAAAAAAAAAAAAZ;\n"},
{"`ifdef FOO\n"
" `fine()\n"
"`else\n"
Expand Down
12 changes: 12 additions & 0 deletions verilog/formatting/token_annotator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,18 @@ static WithReason<SpacingOptions> BreakDecisionBetween(
}
}

if (left.TokenEnum() == PP_define) {
return {SpacingOptions::MustAppend,
"Keep `define and macro name together."};
}
if (right.TokenEnum() == PP_define_body) {
// TODO(b/141517267): reflow macro definition text with flexible
// line-continuations.
return {SpacingOptions::MustAppend,
"Macro definition body must start on same line (but may be "
"line-continued)."};
}

// Check for mandatory line breaks.
if (left.format_token_enum == FTT::eol_comment ||
left.TokenEnum() == PP_define_body // definition excludes trailing '\n'
Expand Down
49 changes: 28 additions & 21 deletions verilog/formatting/token_annotator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) {
{DefaultStyle,
0,
{{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::Undecided}, // FOO
{1, SpacingOptions::MustAppend}, // FOO
{0, SpacingOptions::Undecided}, // (
{0, SpacingOptions::Undecided}, // name
{1, SpacingOptions::Undecided}, // =
Expand Down Expand Up @@ -1086,12 +1086,12 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) {
{DefaultStyle,
1,
{
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::Undecided}, // FOO
{0, SpacingOptions::Undecided}, // "" (empty definition body)
{0, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::Undecided}, // BAR
{0, SpacingOptions::Undecided}, // "" (empty definition body)
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::MustAppend}, // FOO
{0, SpacingOptions::MustAppend}, // "" (empty definition body)
{0, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::MustAppend}, // BAR
{0, SpacingOptions::MustAppend}, // "" (empty definition body)
},
{
{yytokentype::PP_define, "`define"},
Expand All @@ -1107,12 +1107,12 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) {
{DefaultStyle,
1,
{
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::Undecided}, // FOO
{1, SpacingOptions::Undecided}, // 1
{1, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::Undecided}, // BAR
{1, SpacingOptions::Undecided}, // 2
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::MustAppend}, // FOO
{1, SpacingOptions::MustAppend}, // 1
{1, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::MustAppend}, // BAR
{1, SpacingOptions::MustAppend}, // 2
},
{
{yytokentype::PP_define, "`define"},
Expand All @@ -1130,26 +1130,26 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) {
1,
{
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::Undecided}, // FOO
{1, SpacingOptions::MustAppend}, // FOO
{0, SpacingOptions::MustAppend}, // (
{0, SpacingOptions::Undecided}, // )
{0, SpacingOptions::Undecided}, // "" (empty definition body)
{0, SpacingOptions::MustAppend}, // "" (empty definition body)

{0, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::Undecided}, // BAR
{1, SpacingOptions::MustAppend}, // BAR
{0, SpacingOptions::MustAppend}, // (
{0, SpacingOptions::Undecided}, // x
{0, SpacingOptions::Undecided}, // )
{0, SpacingOptions::Undecided}, // "" (empty definition body)
{0, SpacingOptions::MustAppend}, // "" (empty definition body)

{0, SpacingOptions::MustWrap}, // `define
{1, SpacingOptions::Undecided}, // BAZ
{1, SpacingOptions::MustAppend}, // BAZ
{0, SpacingOptions::MustAppend}, // (
{0, SpacingOptions::Undecided}, // y
{0, SpacingOptions::Undecided}, // ,
{1, SpacingOptions::Undecided}, // z
{0, SpacingOptions::Undecided}, // )
{0, SpacingOptions::Undecided}, // "" (empty definition body)
{0, SpacingOptions::MustAppend}, // "" (empty definition body)
},
{
{yytokentype::PP_define, "`define"},
Expand Down Expand Up @@ -1181,13 +1181,13 @@ TEST(TokenAnnotatorTest, AnnotateFormattingInfoTest) {
1,
{
{0, SpacingOptions::Undecided}, // `define
{1, SpacingOptions::Undecided}, // ADD
{1, SpacingOptions::MustAppend}, // ADD
{0, SpacingOptions::MustAppend}, // (
{0, SpacingOptions::Undecided}, // y
{0, SpacingOptions::Undecided}, // ,
{1, SpacingOptions::Undecided}, // z
{0, SpacingOptions::Undecided}, // )
{1, SpacingOptions::Undecided}, // "y+z"
{1, SpacingOptions::MustAppend}, // "y+z"
},
{
{yytokentype::PP_define, "`define"},
Expand Down Expand Up @@ -1977,6 +1977,13 @@ TEST(TokenAnnotatorTest, AnnotateFormattingWithContextTest) {
{}, // any context
{1, SpacingOptions::MustWrap},
},
{
DefaultStyle,
{yytokentype::PP_define, "`define"},
{SymbolIdentifier, "ID"},
{}, // any context
{1, SpacingOptions::MustAppend},
},
{
DefaultStyle,
{TK_StringLiteral, "\"lost/file.svh\""},
Expand Down

0 comments on commit 81f6540

Please sign in to comment.