rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 https://github.com/llvm/llvm-project/issues/60843
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144296
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,24 @@
"int k; }} // namespace out::mid",
Style));
+ verifyFormat("namespace A { namespace B { namespace C {\n"
+ " int i;\n"
+ "}}} // namespace A::B::C\n"
+ "int main() {\n"
+ " if (true)\n"
+ " return 0;\n"
+ "}",
+ "namespace A { namespace B {\n"
+ "namespace C {\n"
+ " int i;\n"
+ "}} // namespace B::C\n"
+ "} // namespace A\n"
+ "int main() {\n"
+ " if (true)\n"
+ " return 0;\n"
+ "}",
+ Style);
+
Style.NamespaceIndentation = FormatStyle::NI_Inner;
EXPECT_EQ("namespace out { namespace in {\n"
" int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -366,20 +366,28 @@
// instead of TheLine->First.
if (Style.CompactNamespaces) {
- if (auto nsToken = TheLine->First->getNamespaceToken()) {
- int i = 0;
- unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
- for (; I + 1 + i != E &&
- nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
- closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
- I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
- // No extra indent for compacted namespaces.
- IndentTracker.skipLine(*I[i + 1]);
+ if (auto *nsToken = TheLine->First->getNamespaceToken()) {
+ int j = 1;
+ unsigned closingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
- Limit -= I[i + 1]->Last->TotalLength;
+ for (; I + j != E &&
+ nsToken->TokenText == getNamespaceTokenText(I[j]) &&
+ closingLineIndex == I[j]->MatchingClosingBlockLineIndex &&
+ I[j]->Last->TotalLength < Limit;
+ j++, --closingLineIndex) {
+ Limit -= I[j]->Last->TotalLength;
+
+ // Reduce indent level for bodies of namespaces which were compacted,
+ // but only if their content was indented in the first place
+ auto *closingLine = AnnotatedLines.begin() + closingLineIndex + 1;
+ auto dedentBy = I[j]->Level - TheLine->Level;
+ for (auto *compactedLine = I + j; compactedLine <= closingLine;
+ compactedLine++) {
+ if (!(*compactedLine)->InPPDirective)
+ (*compactedLine)->Level-= dedentBy;
+ }
}
- return i;
+ return j - 1;
}
if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,24 @@
"int k; }} // namespace out::mid",
Style));
+ verifyFormat("namespace A { namespace B { namespace C {\n"
+ " int i;\n"
+ "}}} // namespace A::B::C\n"
+ "int main() {\n"
+ " if (true)\n"
+ " return 0;\n"
+ "}",
+ "namespace A { namespace B {\n"
+ "namespace C {\n"
+ " int i;\n"
+ "}} // namespace B::C\n"
+ "} // namespace A\n"
+ "int main() {\n"
+ " if (true)\n"
+ " return 0;\n"
+ "}",
+ Style);
+
Style.NamespaceIndentation = FormatStyle::NI_Inner;
EXPECT_EQ("namespace out { namespace in {\n"
" int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -366,20 +366,28 @@
// instead of TheLine->First.
if (Style.CompactNamespaces) {
- if (auto nsToken = TheLine->First->getNamespaceToken()) {
- int i = 0;
- unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
- for (; I + 1 + i != E &&
- nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
- closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
- I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
- // No extra indent for compacted namespaces.
- IndentTracker.skipLine(*I[i + 1]);
+ if (auto *nsToken = TheLine->First->getNamespaceToken()) {
+ int j = 1;
+ unsigned closingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
- Limit -= I[i + 1]->Last->TotalLength;
+ for (; I + j != E &&
+ nsToken->TokenText == getNamespaceTokenText(I[j]) &&
+ closingLineIndex == I[j]->MatchingClosingBlockLineIndex &&
+ I[j]->Last->TotalLength < Limit;
+ j++, --closingLineIndex) {
+ Limit -= I[j]->Last->TotalLength;
+
+ // Reduce indent level for bodies of namespaces which were compacted,
+ // but only if their content was indented in the first place
+ auto *closingLine = AnnotatedLines.begin() + closingLineIndex + 1;
+ auto dedentBy = I[j]->Level - TheLine->Level;
+ for (auto *compactedLine = I + j; compactedLine <= closingLine;
+ compactedLine++) {
+ if (!(*compactedLine)->InPPDirective)
+ (*compactedLine)->Level-= dedentBy;
+ }
}
- return i;
+ return j - 1;
}
if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
