[clang] [clang-format] Fixing erroneous trailing comma (PR #205631)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Barry Revzin (brevzin)

<details>
<summary>Changes</summary>

Fixes #<!-- -->205571 

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


3 Files Affected:

- (modified) clang/lib/Tooling/Core/Replacement.cpp (+6) 
- (modified) clang/unittests/Format/FormatTest.cpp (+13) 
- (modified) clang/unittests/Tooling/RefactoringTest.cpp (+12) 


``````````diff
diff --git a/clang/lib/Tooling/Core/Replacement.cpp b/clang/lib/Tooling/Core/Replacement.cpp
index 10bdc223e33f2..ac5a2aed8111f 100644
--- a/clang/lib/Tooling/Core/Replacement.cpp
+++ b/clang/lib/Tooling/Core/Replacement.cpp
@@ -270,6 +270,12 @@ llvm::Error Replacements::add(const Replacement &R) {
     assert(R.getLength() == 0);
     // `I` is also an insertion, `R` and `I` conflict.
     if (I->getLength() == 0) {
+      // If the two insertions are identical, `R` is redundant; keep the
+      // existing one rather than concatenating. This happens e.g. when
+      // clang-format analyzes the same code under several preprocessor
+      // branches and emits the same insertion in each run.
+      if (R.getReplacementText() == I->getReplacementText())
+        return llvm::Error::success();
       // Check if two insertions are order-independent: if inserting them in
       // either order produces the same text, they are order-independent.
       if ((R.getReplacementText() + I->getReplacementText()).str() !=
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c42cc147cf21e..78ddb27e61e95 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25210,6 +25210,19 @@ TEST_F(FormatTest, EnumTrailingComma) {
                "  MY_ENUM = 0U\n"
                "};",
                Style);
+
+  // Issue https://github.com/llvm/llvm-project/issues/205571
+  verifyFormat("#ifdef FOO\n"
+               "#else\n"
+               "#endif\n"
+               "enum {\n"
+               "  E = 1,\n"
+               "};",
+               "#ifdef FOO\n"
+               "#else\n"
+               "#endif\n"
+               "enum { E = 1 };",
+               Style);
 }
 
 TEST_F(FormatTest, BreakAfterAttributes) {
diff --git a/clang/unittests/Tooling/RefactoringTest.cpp b/clang/unittests/Tooling/RefactoringTest.cpp
index 171dc6de2cae7..7ba989ddb492e 100644
--- a/clang/unittests/Tooling/RefactoringTest.cpp
+++ b/clang/unittests/Tooling/RefactoringTest.cpp
@@ -416,6 +416,18 @@ TEST_F(ReplacementTest, AddInsertAtOtherInsertWhenOderIndependent) {
   EXPECT_EQ(Replacement("x.cc", 10, 3, ""), *std::next(Replaces.begin()));
 }
 
+TEST_F(ReplacementTest, AddIdenticalInsertionsAtSameOffsetDeduplicates) {
+  Replacements Replaces;
+  auto Err = Replaces.add(Replacement("x.cc", 10, 0, ","));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ","));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  EXPECT_EQ(1u, Replaces.size());
+  EXPECT_EQ(Replacement("x.cc", 10, 0, ","), *Replaces.begin());
+}
+
 TEST_F(ReplacementTest, InsertBetweenAdjacentReplacements) {
   Replacements Replaces;
   auto Err = Replaces.add(Replacement("x.cc", 10, 5, "a"));

``````````

</details>


https://github.com/llvm/llvm-project/pull/205631