darwin updated this revision to Diff 353200.
darwin added a comment.
Updated with the fix code.
Look into the code, I am pretty sure this is a bug, it is about the logic of
the parameter `KeepEmptyLinesAtTheStartOfBlocks`.
When `KeepEmptyLinesAtTheStartOfBlocks` is false, it will remove the empty
lines at the start of the block, and namespace will be an exception. So the
empty lines will be kept inside namespace.
The problem is, it can only handle the situation in which the `namespace` and
the `{` are on the same line. If `BraceWrapping.AfterNamespace` is true, it
will cause the `namespace` and the `{` to be separated into different lines.
The original code overlooked this situation:
// Remove empty lines after "{".
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
PreviousLine->Last->is(tok::l_brace) &&
!PreviousLine->startsWithNamespace() &&
!startsExternCBlock(*PreviousLine))
Newlines = 1;
As you can see, it only checks if previous line starts with namespace.
The solution is a bit of tricky, we need to check not only the **previous
line**, but also the **line before the previous line**. There isn't an easy way
to get this. So I added a new parameter `PrevPrevLine` to the
`UnwrappedLineFormatter::formatFirstToken()` function. I have to admit this
isn't an elegant solution. Let me know if there is a better way to do so.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104044/new/
https://reviews.llvm.org/D104044
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineFormatter.h
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -262,6 +262,89 @@
"}",
getGoogleStyle()));
+ auto CustomStyle = clang::format::getLLVMStyle();
+ CustomStyle.BreakBeforeBraces = clang::format::FormatStyle::BS_Custom;
+ CustomStyle.BraceWrapping.AfterNamespace = true;
+ CustomStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
+ EXPECT_EQ("namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ format("namespace N\n"
+ "{\n"
+ "\n"
+ "\n"
+ "int i;\n"
+ "}",
+ CustomStyle));
+ EXPECT_EQ("/* something */ namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ format("/* something */ namespace N {\n"
+ "\n"
+ "\n"
+ "int i;\n"
+ "}",
+ CustomStyle));
+ EXPECT_EQ("inline namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ format("inline namespace N\n"
+ "{\n"
+ "\n"
+ "\n"
+ "int i;\n"
+ "}",
+ CustomStyle));
+ EXPECT_EQ("/* something */ inline namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ format("/* something */ inline namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ CustomStyle));
+ EXPECT_EQ("export namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ format("export namespace N\n"
+ "{\n"
+ "\n"
+ "int i;\n"
+ "}",
+ CustomStyle));
+ EXPECT_EQ("namespace a\n"
+ "{\n"
+ "namespace b\n"
+ "{\n"
+ "\n"
+ "class AA {};\n"
+ "\n"
+ "} // namespace b\n"
+ "} // namespace a\n",
+ format("namespace a\n"
+ "{\n"
+ "namespace b\n"
+ "{\n"
+ "\n"
+ "\n"
+ "class AA {};\n"
+ "\n"
+ "\n"
+ "}\n"
+ "}\n",
+ CustomStyle));
+
// ...but do keep inlining and removing empty lines for non-block extern "C"
// functions.
verifyFormat("extern \"C\" int f() { return 42; }", getGoogleStyle());
Index: clang/lib/Format/UnwrappedLineFormatter.h
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.h
+++ clang/lib/Format/UnwrappedLineFormatter.h
@@ -47,6 +47,7 @@
/// of the \c UnwrappedLine if there was no structural parsing error.
void formatFirstToken(const AnnotatedLine &Line,
const AnnotatedLine *PreviousLine,
+ const AnnotatedLine *PrevPrevLine,
const SmallVectorImpl<AnnotatedLine *> &Lines,
unsigned Indent, unsigned NewlineIndent);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1122,6 +1122,7 @@
unsigned Penalty = 0;
LevelIndentTracker IndentTracker(Style, Keywords, Lines[0]->Level,
AdditionalIndent);
+ const AnnotatedLine *PrevPrevLine = nullptr;
const AnnotatedLine *PreviousLine = nullptr;
const AnnotatedLine *NextLine = nullptr;
@@ -1160,7 +1161,7 @@
if (ShouldFormat && TheLine.Type != LT_Invalid) {
if (!DryRun) {
bool LastLine = Line->First->is(tok::eof);
- formatFirstToken(TheLine, PreviousLine, Lines, Indent,
+ formatFirstToken(TheLine, PreviousLine, PrevPrevLine, Lines, Indent,
LastLine ? LastStartColumn : NextStartColumn + Indent);
}
@@ -1206,7 +1207,7 @@
TheLine.LeadingEmptyLinesAffected);
// Format the first token.
if (ReformatLeadingWhitespace)
- formatFirstToken(TheLine, PreviousLine, Lines,
+ formatFirstToken(TheLine, PreviousLine, PrevPrevLine, Lines,
TheLine.First->OriginalColumn,
TheLine.First->OriginalColumn);
else
@@ -1222,6 +1223,7 @@
}
if (!DryRun)
markFinalized(TheLine.First);
+ PrevPrevLine = PreviousLine;
PreviousLine = &TheLine;
}
PenaltyCache[CacheKey] = Penalty;
@@ -1230,6 +1232,7 @@
void UnwrappedLineFormatter::formatFirstToken(
const AnnotatedLine &Line, const AnnotatedLine *PreviousLine,
+ const AnnotatedLine *PrevPrevLine,
const SmallVectorImpl<AnnotatedLine *> &Lines, unsigned Indent,
unsigned NewlineIndent) {
FormatToken &RootToken = *Line.First;
@@ -1261,6 +1264,8 @@
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
PreviousLine->Last->is(tok::l_brace) &&
!PreviousLine->startsWithNamespace() &&
+ !(PrevPrevLine && PrevPrevLine->startsWithNamespace() &&
+ PreviousLine->startsWith(tok::l_brace)) &&
!startsExternCBlock(*PreviousLine))
Newlines = 1;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
