Skip to content

Commit 7ba9101

Browse files
committed
[clang-format] Rewrite how indent is reduced for compacted namespaces
The previous version set the indentation directly using IndentForLevel, however, this has a few caveats, namely: * IndentForLevel applies to all scopes of the entire program being formatted, but this indentation should only be adjusted for scopes of namespaces. * The method it used only set the correct indent amount if one wasn't already set for a given level, meaning it didn't work correctly if anything with indentation preceded a namespace keyword. This includes preprocessing directives if using IndentPPDirectives. This patch instead reduces the Level of all lines within namespaces which are compacted by CompactNamespaces. This means they will get correct indentation using the normal process. Fixes #60843 Reviewed By: owenpan, MyDeveloperDay, HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D144296
1 parent b0676fb commit 7ba9101

File tree

2 files changed

+62
-21
lines changed

2 files changed

+62
-21
lines changed

clang/lib/Format/UnwrappedLineFormatter.cpp

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ class LevelIndentTracker {
6060
Offset = getIndentOffset(*Line.First);
6161
// Update the indent level cache size so that we can rely on it
6262
// having the right size in adjustToUnmodifiedline.
63-
skipLine(Line, /*UnknownIndent=*/true);
63+
if (Line.Level >= IndentForLevel.size())
64+
IndentForLevel.resize(Line.Level + 1, -1);
6465
if (Style.IndentPPDirectives != FormatStyle::PPDIS_None &&
6566
(Line.InPPDirective ||
6667
(Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
@@ -81,13 +82,6 @@ class LevelIndentTracker {
8182
Indent = Line.Level * Style.IndentWidth + Style.ContinuationIndentWidth;
8283
}
8384

84-
/// Update the indent state given that \p Line indent should be
85-
/// skipped.
86-
void skipLine(const AnnotatedLine &Line, bool UnknownIndent = false) {
87-
if (Line.Level >= IndentForLevel.size())
88-
IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
89-
}
90-
9185
/// Update the level indent to adapt to the given \p Line.
9286
///
9387
/// When a line is not formatted, we move the subsequent lines on the same
@@ -367,20 +361,27 @@ class LineJoiner {
367361
// instead of TheLine->First.
368362

369363
if (Style.CompactNamespaces) {
370-
if (auto nsToken = TheLine->First->getNamespaceToken()) {
371-
int i = 0;
372-
unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
373-
for (; I + 1 + i != E &&
374-
nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
375-
closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
376-
I[i + 1]->Last->TotalLength < Limit;
377-
i++, --closingLine) {
378-
// No extra indent for compacted namespaces.
379-
IndentTracker.skipLine(*I[i + 1]);
380-
381-
Limit -= I[i + 1]->Last->TotalLength;
364+
if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
365+
int J = 1;
366+
assert(TheLine->MatchingClosingBlockLineIndex > 0);
367+
for (auto ClosingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
368+
I + J != E && NSToken->TokenText == getNamespaceTokenText(I[J]) &&
369+
ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
370+
I[J]->Last->TotalLength < Limit;
371+
++J, --ClosingLineIndex) {
372+
Limit -= I[J]->Last->TotalLength;
373+
374+
// Reduce indent level for bodies of namespaces which were compacted,
375+
// but only if their content was indented in the first place.
376+
auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
377+
auto OutdentBy = I[J]->Level - TheLine->Level;
378+
for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
379+
++CompactedLine) {
380+
if (!(*CompactedLine)->InPPDirective)
381+
(*CompactedLine)->Level -= OutdentBy;
382+
}
382383
}
383-
return i;
384+
return J - 1;
384385
}
385386

386387
if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {

clang/unittests/Format/FormatTest.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4431,6 +4431,46 @@ TEST_F(FormatTest, FormatsCompactNamespaces) {
44314431
"int k; }} // namespace out::mid",
44324432
Style));
44334433

4434+
verifyFormat("namespace A { namespace B { namespace C {\n"
4435+
" int i;\n"
4436+
"}}} // namespace A::B::C\n"
4437+
"int main() {\n"
4438+
" if (true)\n"
4439+
" return 0;\n"
4440+
"}",
4441+
"namespace A { namespace B {\n"
4442+
"namespace C {\n"
4443+
" int i;\n"
4444+
"}} // namespace B::C\n"
4445+
"} // namespace A\n"
4446+
"int main() {\n"
4447+
" if (true)\n"
4448+
" return 0;\n"
4449+
"}",
4450+
Style);
4451+
4452+
verifyFormat("namespace A { namespace B { namespace C {\n"
4453+
"#ifdef FOO\n"
4454+
" int i;\n"
4455+
"#endif\n"
4456+
"}}} // namespace A::B::C\n"
4457+
"int main() {\n"
4458+
" if (true)\n"
4459+
" return 0;\n"
4460+
"}",
4461+
"namespace A { namespace B {\n"
4462+
"namespace C {\n"
4463+
"#ifdef FOO\n"
4464+
" int i;\n"
4465+
"#endif\n"
4466+
"}} // namespace B::C\n"
4467+
"} // namespace A\n"
4468+
"int main() {\n"
4469+
" if (true)\n"
4470+
" return 0;\n"
4471+
"}",
4472+
Style);
4473+
44344474
Style.NamespaceIndentation = FormatStyle::NI_Inner;
44354475
EXPECT_EQ("namespace out { namespace in {\n"
44364476
" int i;\n"

0 commit comments

Comments
 (0)