Skip to content

Conversation

@owenca
Copy link
Contributor

@owenca owenca commented Dec 20, 2024

Fixes #109864.

@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #109864.


Full diff: https://github.com/llvm/llvm-project/pull/120680.diff

2 Files Affected:

  • (modified) clang/lib/Format/Format.cpp (+8-1)
  • (modified) clang/unittests/Format/SortIncludesTest.cpp (+12)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..f79f9cb1ecdd22 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3247,7 +3247,14 @@ tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
   std::string RawStringTermination = ")\"";
 
   for (;;) {
-    auto Pos = Code.find('\n', SearchFrom);
+    size_t Pos = SearchFrom;
+    if (Code[SearchFrom] != '\n') {
+      do { // Search for the first newline while skipping line splices.
+        ++Pos;
+        Pos = Code.find('\n', Pos);
+      } while (Pos != StringRef::npos && Code[Pos - 1] == '\\');
+    }
+
     StringRef Line =
         Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp
index 31753825646373..cb3f8c73a04871 100644
--- a/clang/unittests/Format/SortIncludesTest.cpp
+++ b/clang/unittests/Format/SortIncludesTest.cpp
@@ -984,6 +984,18 @@ TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) {
                     "#include <c>\n"
                     "#include <b>"));
 
+  verifyFormat("/* COPYRIGHT *\\\n"
+               "\\* (C) 2024  */\n"
+               "\n"
+               "#include <a>\n"
+               "#include <b>",
+               sort("/* COPYRIGHT *\\\n"
+                    "\\* (C) 2024  */\n"
+                    "\n"
+                    "#include <b>\n"
+                    "#include <a>\n"
+                    "#include <b>"));
+
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
   verifyFormat("#include <a>\n"
                "#include <b>\n"


for (;;) {
auto Pos = Code.find('\n', SearchFrom);
for (const auto Size = Code.size(); SearchFrom < Size;) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the extra Size variable needed? Why can't it stay as for (;;) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's used in SearchFrom < Size. My previous version using for (;;) caused a crash in ToolingTests.

@owenca owenca merged commit 141c544 into llvm:main Dec 25, 2024
8 checks passed
@owenca owenca deleted the 109864 branch December 25, 2024 05:47
@llvm llvm deleted a comment from llvm-ci Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang-format] Regression: Include sorting is broken with backslashes in multiline comments.

4 participants