This is an archive of the discontinued LLVM Phabricator instance.

Differential D42298

[clang-format] Fix shortening blocks in macros causing merged next line
ClosedPublic

Authored by krasimir on Jan 19 2018, 6:34 AM.

Details

Reviewers
bkramer
Commits
rGbf4cddaafb1e: [clang-format] Fix shortening blocks in macros causing merged next line
rC322954: [clang-format] Fix shortening blocks in macros causing merged next line
rL322954: [clang-format] Fix shortening blocks in macros causing merged next line
Summary

This patch addresses bug 36002, where a combination of options causes the line
following a short block in macro to be merged with that macro.

Diff Detail

Event Timeline

krasimir created this revision.Jan 19 2018, 6:34 AM
Herald added a subscriber: klimek. · View Herald TranscriptJan 19 2018, 6:34 AM
Harbormaster completed remote builds in B14022: Diff 130603.Jan 19 2018, 6:34 AM
krasimir updated this revision to Diff 130604.Jan 19 2018, 6:35 AM
  • Remove added newline
Harbormaster completed remote builds in B14023: Diff 130604.Jan 19 2018, 6:36 AM
krasimir updated this revision to Diff 130609.Jan 19 2018, 6:40 AM
  • Remove double whitespace
Harbormaster completed remote builds in B14024: Diff 130609.Jan 19 2018, 6:40 AM
bkramer accepted this revision.Jan 19 2018, 6:43 AM

lgtm

lib/Format/UnwrappedLineFormatter.cpp
310

double space in comment

This revision is now accepted and ready to land.Jan 19 2018, 6:43 AM
krasimir marked an inline comment as done.Jan 19 2018, 8:07 AM
Closed by commit rL322954: [clang-format] Fix shortening blocks in macros causing merged next line (authored by krasimir). · Explain WhyJan 19 2018, 8:14 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptJan 19 2018, 8:14 AM

Revision Contents

PathSize
lib/
Format/
12 lines
unittests/
Format/
17 lines

Diff 130604

lib/Format/UnwrappedLineFormatter.cpp

Show First 20 Lines • Show All 298 Lines • ▼ Show 20 Lines if (I[1]->First->is(tok::l_brace) &&
? tryMergeSimpleBlock(I, E, Limit) ? tryMergeSimpleBlock(I, E, Limit)
: 0; : 0;
} }
// Try to merge either empty or one-line block if is precedeed by control // Try to merge either empty or one-line block if is precedeed by control
// statement token // statement token
if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last && if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
I != AnnotatedLines.begin() && I != AnnotatedLines.begin() &&
I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) { I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
return Style.AllowShortBlocksOnASingleLine unsigned MergedLines = 0;
? tryMergeSimpleBlock(I - 1, E, Limit) if (Style.AllowShortBlocksOnASingleLine) {
: 0; MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
// If we managed to merge the block, discard the first merged line
bkramerUnsubmitted
Done
ReplyInline Actions

double space in comment

bkramer: double space in comment
// since we are merging starting from I.
if (MergedLines > 0)
--MergedLines;
}
return MergedLines;
} }
// Try to merge a block with left brace wrapped that wasn't yet covered // Try to merge a block with left brace wrapped that wasn't yet covered
if (TheLine->Last->is(tok::l_brace)) { if (TheLine->Last->is(tok::l_brace)) {
return !Style.BraceWrapping.AfterFunction || return !Style.BraceWrapping.AfterFunction ||
(I[1]->First->is(tok::r_brace) && (I[1]->First->is(tok::r_brace) &&
!Style.BraceWrapping.SplitEmptyRecord) !Style.BraceWrapping.SplitEmptyRecord)
? tryMergeSimpleBlock(I, E, Limit) ? tryMergeSimpleBlock(I, E, Limit)
: 0; : 0;
▲ Show 20 Lines • Show All 863 Lines • Show Last 20 Lines

unittests/Format/FormatTest.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 582 Lines • ▼ Show 20 LinesTEST_F(FormatTest, FormatShortBracedStatements) {
verifyFormat("for (;;) {}", AllowSimpleBracedStatements); verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
verifyFormat("for (;;)\n" verifyFormat("for (;;)\n"
"{\n" "{\n"
" f();\n" " f();\n"
"}", "}",
AllowSimpleBracedStatements); AllowSimpleBracedStatements);
} }
TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
FormatStyle Style = getLLVMStyleWithColumns(60);
Style.AllowShortBlocksOnASingleLine = true;
Style.AllowShortIfStatementsOnASingleLine = true;
Style.BreakBeforeBraces = FormatStyle::BS_Allman;
EXPECT_EQ("#define A \\\n"
" if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
" { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
"X;",
format("#define A \\\n"
" if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
" RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
" }\n"
"X;",
Style));
}
TEST_F(FormatTest, ParseIfElse) { TEST_F(FormatTest, ParseIfElse) {
verifyFormat("if (true)\n" verifyFormat("if (true)\n"
" if (true)\n" " if (true)\n"
" if (true)\n" " if (true)\n"
" f();\n" " f();\n"
" else\n" " else\n"
" g();\n" " g();\n"
" else\n" " else\n"
▲ Show 20 Lines • Show All 11,261 Lines • Show Last 20 Lines