Sedeniono updated this revision to Diff 539583.
Sedeniono added a comment.
@owenpan Oh wow, you are right. Phabricator shows that there are two separate
commits, under "Revision Contents" > "Commits", and their commit messages. But
apparently there is no way to download some proper *.patch file that contains
the meta data, only the total diff.
So, I uploaded a new diff here (D151047 <https://reviews.llvm.org/D151047>): It
contains the bugfix. I will also edit the summary at the top with the
corresponding commit message.
I have submitted the refactoring commit separately here:
https://reviews.llvm.org/D155094
Note that both patches (D151047 <https://reviews.llvm.org/D151047> and D155094
<https://reviews.llvm.org/D155094>) are fortunately independent of each other.
I checked that both can be applied to the latest 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)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
