Sedeniono updated this revision to Diff 534688.
Sedeniono added a comment.
As suggested by @owenpan, split the commits differently. My previous statement,
that the whole fix consists of a revert of D129064
<https://reviews.llvm.org/D129064> was not entirely correct: The fix
reintroduces the `resize()` that got removed in D129064
<https://reviews.llvm.org/D129064>, but it additionally surrounds it by the `if
(!Line.InPPDirective)`. Otherwise, the formatting is wrong if some PP
directives are encountered. (The FormatMacroRegardlessOfPreviousIndent tests
check this.)
Also, as @owenpan suggested, I replaced the `!Line.First->is(tok::comment)`
with `Line.First->isNot(tok::comment)` in `adjustToUnmodifiedLine()`.
I also rebased the patch to the current main branch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151047/new/
https://reviews.llvm.org/D151047
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTestSelective.cpp
Index: clang/unittests/Format/FormatTestSelective.cpp
===================================================================
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@
format(" int a;\n"
"void ffffff() {}",
11, 0));
+
+ // https://github.com/llvm/llvm-project/issues/59178
+ Style = getMozillaStyle();
+ EXPECT_EQ("int a()\n"
+ "{\n"
+ "return 0;\n"
+ "}\n"
+ "int b()\n"
+ "{\n"
+ " return 42;\n"
+ "}",
+ format("int a()\n"
+ "{\n"
+ "return 0;\n"
+ "}\n"
+ "int b()\n"
+ "{\n"
+ "return 42;\n" // Format this line only
+ "}",
+ 32, 0));
}
TEST_F(FormatTestSelective, UnderstandsTabs) {
@@ -617,6 +637,70 @@
"namespace ns1 { namespace ns2 {\n"
"}}";
EXPECT_EQ(Code, format(Code, 0, 0));
+
+ Style = getLLVMStyle();
+ Style.FixNamespaceComments = false;
+ Code = "namespace ns {\n"
+ "#define REF(alias) alias alias_var;\n"
+ "}";
+ EXPECT_EQ(Code, format(Code, 51, 1));
+}
+
+TEST_F(FormatTestSelective, FormatMacroRegardlessOfPreviousIndent) {
+ // clang-format currently does not (or should not) take into account the
+ // indent of previous unformatted lines when formatting a PP directive.
+ // Technically speaking, LevelIndentTracker::IndentForLevel is only for non-PP
+ // lines. So these tests here check that the indent of previous non-PP lines
+ // do not affect the formatting. If this requirement changes, the tests here
+ // need to be adapted.
+ Style = getLLVMStyle();
+
+ Style.IndentPPDirectives = FormatStyle::PPDirectiveIndentStyle::PPDIS_None;
+ EXPECT_EQ(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ "#define some\n" // Formatted line
+ "#endif\n" // That this line is also formatted might be a bug.
+ " }};", // Dito: Bug?
+ format(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ " #define some\n" // format this line
+ " #endif\n"
+ " }};",
+ 75, 0));
+
+ Style.IndentPPDirectives =
+ FormatStyle::PPDirectiveIndentStyle::PPDIS_BeforeHash;
+ EXPECT_EQ(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ " #define some\n" // Formatted line
+ " #endif\n"
+ " }};",
+ format(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ " #define some\n" // format this line
+ " #endif\n"
+ " }};",
+ 75, 0));
+
+ Style.IndentPPDirectives =
+ FormatStyle::PPDirectiveIndentStyle::PPDIS_AfterHash;
+ EXPECT_EQ(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ "# define some\n" // Formatted line
+ "#endif\n" // That this line is also formatted might be a bug.
+ " }};",
+ format(" class Foo {\n"
+ " void test() {\n"
+ " #ifdef 1\n"
+ " #define some\n" // format this line
+ " #endif\n"
+ " }};",
+ 75, 0));
}
} // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -74,6 +74,13 @@
: Line.Level * PPIndentWidth;
Indent += AdditionalIndent;
} else {
+ // When going to lower levels, forget previous higher levels so that we
+ // recompute future higher levels. But don't forget them if we enter a PP
+ // directive, since these do not terminate a C++ code block.
+ if (!Line.InPPDirective) {
+ assert(Line.Level <= IndentForLevel.size());
+ IndentForLevel.resize(Line.Level + 1);
+ }
Indent = getIndent(Line.Level);
}
if (static_cast<int>(Indent) + Offset >= 0)
@@ -88,13 +95,13 @@
/// level to the same indent.
/// Note that \c nextLine must have been called before this method.
void adjustToUnmodifiedLine(const AnnotatedLine &Line) {
- unsigned LevelIndent = Line.First->OriginalColumn;
- if (static_cast<int>(LevelIndent) - Offset >= 0)
- LevelIndent -= Offset;
- assert(Line.Level < IndentForLevel.size());
- if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
- !Line.InPPDirective) {
- IndentForLevel[Line.Level] = LevelIndent;
+ if (!Line.InPPDirective) {
+ unsigned LevelIndent = Line.First->OriginalColumn;
+ if (static_cast<int>(LevelIndent) - Offset >= 0)
+ LevelIndent -= Offset;
+ assert(Line.Level < IndentForLevel.size());
+ if (Line.First->isNot(tok::comment) || IndentForLevel[Line.Level] == -1)
+ IndentForLevel[Line.Level] = LevelIndent;
}
}
@@ -148,6 +155,7 @@
/// at \p IndentForLevel[l], or a value < 0 if the indent for
/// that level is unknown.
unsigned getIndent(unsigned Level) const {
+ assert(Level < IndentForLevel.size());
if (IndentForLevel[Level] != -1)
return IndentForLevel[Level];
if (Level == 0)
@@ -159,7 +167,10 @@
const AdditionalKeywords &Keywords;
const unsigned AdditionalIndent;
- /// The indent in characters for each level.
+ /// The indent in characters for each level. It remembers the indent of
+ /// previous lines (that are not PP directives) of equal or lower levels. This
+ /// is used to align formatted lines to the indent of previous non-formatted
+ /// lines. Think about the --lines parameter of clang-format.
SmallVector<int> IndentForLevel;
/// Offset of the current line relative to the indent level.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
