Skip to content

[clang-format] Finalize children after formatting them #73753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Nov 29, 2023

This would also fix the overlapping replacements below:

$ clang-format
 a(
 #else
 #endif
) = []() {      
)}
The new replacement overlaps with an existing replacement.
New replacement: <stdin>: 38:+7:"
"
Existing replacement: <stdin>: 38:+7:" "

Fixed #73487.

This would also fix the overlapping replacements below:
$ clang-format
 a(
 #else
 #endif
) = []() {
)}
The new replacement overlaps with an existing replacement.
New replacement: <stdin>: 38:+7:"
"
Existing replacement: <stdin>: 38:+7:" "

Fixed llvm#73487.
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2023

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

This would also fix the overlapping replacements below: $ clang-format
a(
#else
#endif
) = {
)}
The new replacement overlaps with an existing replacement. New replacement: <stdin>: 38:+7:"
"
Existing replacement: <stdin>: 38:+7:" "

Fixed #73487.


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

2 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+10-10)
  • (modified) clang/unittests/Format/FormatTest.cpp (+7)
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 280485d9a90d1bf..40730cd53529ed3 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -939,6 +939,12 @@ class LineJoiner {
 };
 
 static void markFinalized(FormatToken *Tok) {
+  if (Tok->is(tok::hash) && !Tok->Previous && Tok->Next &&
+      Tok->Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+                         tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+                         tok::pp_else, tok::pp_endif)) {
+    Tok = Tok->Next;
+  }
   for (; Tok; Tok = Tok->Next) {
     if (Tok->MacroCtx && Tok->MacroCtx->Role == MR_ExpandedArg) {
       // In the first pass we format all macro arguments in the expanded token
@@ -1060,6 +1066,8 @@ class LineFormatter {
     }
     Penalty +=
         formatLine(*Child, State.Column + 1, /*FirstStartColumn=*/0, DryRun);
+    if (!DryRun)
+      markFinalized(Child->First);
 
     State.Column += 1 + Child->Last->TotalLength;
     return true;
@@ -1429,16 +1437,8 @@ unsigned UnwrappedLineFormatter::format(
       NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
       RangeMinLevel = UINT_MAX;
     }
-    if (!DryRun) {
-      auto *Tok = TheLine.First;
-      if (Tok->is(tok::hash) && !Tok->Previous && Tok->Next &&
-          Tok->Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
-                             tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
-                             tok::pp_else, tok::pp_endif)) {
-        Tok = Tok->Next;
-      }
-      markFinalized(Tok);
-    }
+    if (!DryRun)
+      markFinalized(TheLine.First);
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 420afe5992f2a0b..8782cb7c49cad33 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6361,6 +6361,13 @@ TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
                "  int   quux      = 4;\n"
                "}",
                Style);
+  verifyFormat("auto foo = [] { return; };\n"
+               "#if FOO\n"
+               "#else\n"
+               "count = bar;\n"
+               "mbid  = bid;\n"
+               "#endif",
+               Style);
 
   // Test with a mix of #if and #else blocks.
   verifyFormat("void f1() {\n"

@owenca owenca merged commit bbae59a into llvm:main Nov 29, 2023
@owenca owenca deleted the 73487 branch November 29, 2023 20:56
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Nov 30, 2023
Local branch amd-gfx af6b7b0 Merged main:2f1399c73f52 into amd-gfx:88b84f4b16eb
Remote branch main bbae59a [clang-format] Finalize children after formatting them (llvm#73753)
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 in clang-format version 17
3 participants