[clang] [clang-format] Defer line merging to the unexpanded pass (PR #233944)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 02:07:49 PDT 2026
https://github.com/r4nt updated https://github.com/llvm/llvm-project/pull/233944
>From a59a515c286d40e2cdba514ad8029a6d92fcb5bc Mon Sep 17 00:00:00 2001
From: Manuel Klimek <klimek at google.com>
Date: Tue, 20 Oct 2026 17:03:38 +0200
Subject: [PATCH] [clang-format] Defer line merging to the unexpanded pass when
expanding macros
When Configured Macro Expansion (Style.Macros) is used, clang-format runs two
formatting passes: Run 0 formats the macro-expanded UnwrappedLines (and finalizes
all non-macro-expanded lines and macro argument tokens), and Run 1 formats the
reconstructed unexpanded UnwrappedLines.
Previously, LineJoiner ran in both Run 0 and Run 1, and Run 0 finalized the
first token of every top-level UnwrappedLine. This caused several bugs:
1. If a macro expanded to multiple UnwrappedLines whose first line could merge
with a preceding header in Run 0 (e.g. `void f() { IF_ERROR_RETURN(x); }`
where `IF_ERROR_RETURN(x)` expands to `if (x)\n return x;` with
`AllowShortIfStatementsOnASingleLine: Never` and `AllowShortFunctionsOnASingleLine: All`,
or `FN=class C { int f` in `void f() { FN(a *b); }; }`), Run 0 merged the
first expanded line onto `void f() {` and finalized the macro token's
whitespace, while leaving `}` on a subsequent line.
2. Conversely, if a macro call's expanded line was longer than the unexpanded
macro call (preventing a merge in Run 0), finalizing the first token of each
top-level line in Run 0 prevented LineJoiner in Run 1 from merging lines
that fit on a single line in the unexpanded code (or conversely, Run 0
merged lines whose unexpanded macro call exceeded the column limit in Run 1).
3. In MacroCallReconstructor::finalize(), multi-line object-like macros where
subsequent expanded lines contained only hidden macro tokens (e.g.
`STMTS=f(); g();`) produced empty child ReconstructedLines, crashing on
`Last->Children.back()->Tokens.back()` when `Tokens` was empty.
Fix this by:
- Skipping line merging (`LineJoiner`) and not finalizing the first token of
top-level UnwrappedLines during Run 0 (`InMacroExpansion`), while still
transitioning `MR_ExpandedArg` to `MR_UnexpandedArg` if the first token is a
macro argument.
- Running `LineJoiner` in Run 1 using the unexpanded token lengths, and
disallowing merges across lines that contain multi-line macro expansions or
internal line breaks from Run 0.
- Skipping empty child lines in `MacroCallReconstructor::finalize()` after
marking `Last->Tok->MacroParent = true`, and marking `Reconstructed`'s last
token as `MacroParent` whenever a macro call expands to multiple
`UnwrappedLine`s.
---
clang/lib/Format/MacroCallReconstructor.cpp | 7 ++-
clang/lib/Format/TokenAnnotator.h | 5 +-
clang/lib/Format/UnwrappedLineFormatter.cpp | 48 ++++++++++++----
clang/lib/Format/UnwrappedLineParser.cpp | 9 ++-
clang/lib/Format/UnwrappedLineParser.h | 3 +
clang/lib/Format/WhitespaceManager.cpp | 20 +++----
clang/lib/Format/WhitespaceManager.h | 3 +-
.../Format/FormatTestMacroExpansion.cpp | 57 ++++++++++++++++++-
8 files changed, 122 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Format/MacroCallReconstructor.cpp b/clang/lib/Format/MacroCallReconstructor.cpp
index 895d9f93dfce38..f62e208043e890 100644
--- a/clang/lib/Format/MacroCallReconstructor.cpp
+++ b/clang/lib/Format/MacroCallReconstructor.cpp
@@ -484,12 +484,13 @@ void MacroCallReconstructor::finalize() {
LineNode *Last = (*I)->Tokens.back().get();
++I;
for (auto *E = Top.Children.end(); I != E; ++I) {
- assert(Last->Children.empty());
- Last->Children.push_back(std::move(*I));
-
// Mark the previous line's last token as generated by a macro expansion
// so the formatting algorithm can take that into account.
Last->Tok->MacroParent = true;
+ if ((*I)->Tokens.empty())
+ continue;
+ assert(Last->Children.empty());
+ Last->Children.push_back(std::move(*I));
Last = Last->Children.back()->Tokens.back().get();
}
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index c909c39e92ea3d..57d479a99f92b7 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -57,7 +57,7 @@ class AnnotatedLine {
MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
InPPDirective(Line.InPPDirective),
InPragmaDirective(Line.InPragmaDirective),
- InMacroBody(Line.InMacroBody),
+ InMacroBody(Line.InMacroBody), InMacroExpansion(Line.InMacroExpansion),
IsModuleOrImportDecl(Line.IsModuleOrImportDecl),
MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
IsMultiVariableDeclStmt(false), Affected(false),
@@ -70,6 +70,8 @@ class AnnotatedLine {
// Next and Previous for every token, as previous formatting runs might have
// left them in a different state.
First->Previous = nullptr;
+ if (First->MacroParent)
+ ContainsMacroCall = true;
FormatToken *Current = First;
addChildren(Line.Tokens.front(), Current);
for (const UnwrappedLineNode &Node : llvm::drop_begin(Line.Tokens)) {
@@ -180,6 +182,7 @@ class AnnotatedLine {
bool InPPDirective;
bool InPragmaDirective;
bool InMacroBody;
+ bool InMacroExpansion;
bool IsModuleOrImportDecl;
bool MustBeDeclaration;
bool MightBeFunctionDecl;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index e42a2083f7268a..304d8649f2e819 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -215,7 +215,10 @@ class LineJoiner {
return nullptr;
const AnnotatedLine *Current = *Next;
IndentTracker.nextLine(*Current);
- unsigned MergedLines = tryFitMultipleLinesInOne(IndentTracker, Next, End);
+ unsigned MergedLines =
+ Current->InMacroExpansion
+ ? 0
+ : tryFitMultipleLinesInOne(IndentTracker, Next, End);
if (MergedLines > 0 && Style.ColumnLimit == 0) {
// Disallow line merging if there is a break at the start of one of the
// input lines.
@@ -223,6 +226,12 @@ class LineJoiner {
if (Next[i + 1]->First->NewlinesBefore > 0)
MergedLines = 0;
}
+ for (unsigned i = 0; i <= MergedLines; ++i) {
+ if (cannotMergeMacroLine(Next[i])) {
+ MergedLines = 0;
+ break;
+ }
+ }
if (!DryRun)
for (unsigned i = 0; i < MergedLines; ++i)
join(*Next[0], *Next[i + 1]);
@@ -1109,6 +1118,19 @@ class LineJoiner {
return false;
}
+ bool cannotMergeMacroLine(const AnnotatedLine *Line) {
+ assert(Line->First);
+ if (Line->Last->MacroParent)
+ return true;
+ for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
+ if (Tok->MacroParent && !Tok->Children.empty())
+ return true;
+ if (Tok != Line->First && Tok->getDecision() == FD_Break)
+ return true;
+ }
+ return false;
+ }
+
void join(AnnotatedLine &A, const AnnotatedLine &B) {
assert(!A.Last->Next);
assert(!B.First->Previous);
@@ -1134,14 +1156,15 @@ class LineJoiner {
const SmallVectorImpl<AnnotatedLine *> &AnnotatedLines;
};
-static void markFinalized(FormatToken *Tok) {
+static void markFinalized(FormatToken *Tok, bool SkipFirst = false) {
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;
+ SkipFirst = false;
}
- for (; Tok; Tok = Tok->Next) {
+ for (bool First = true; Tok; Tok = Tok->Next, First = false) {
if (Tok->MacroCtx && Tok->MacroCtx->Role == MR_ExpandedArg) {
// In the first pass we format all macro arguments in the expanded token
// stream. Instead of finalizing the macro arguments, we mark that they
@@ -1157,7 +1180,7 @@ static void markFinalized(FormatToken *Tok) {
Tok->SpacesRequiredBefore = 0;
if (!Tok->MustBreakBeforeFinalized)
Tok->MustBreakBefore = 0;
- } else {
+ } else if (!First || !SkipFirst) {
Tok->Finalized = true;
}
}
@@ -1636,7 +1659,7 @@ unsigned UnwrappedLineFormatter::format(
RangeMinLevel = UINT_MAX;
}
if (!DryRun)
- markFinalized(TheLine.First);
+ markFinalized(TheLine.First, /*SkipFirst=*/TheLine.InMacroExpansion);
}
PenaltyCache[CacheKey] = Penalty;
return Penalty;
@@ -1764,8 +1787,11 @@ void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line,
RootToken.NewlinesBefore,
Style.KeepEmptyLines.AtEndOfFile ? Style.MaxEmptyLinesToKeep + 1 : 1);
unsigned TokenIndent = Newlines ? NewlineIndent : 0;
- Whitespaces->replaceWhitespace(RootToken, Newlines, TokenIndent,
- TokenIndent);
+ Whitespaces->replaceWhitespace(
+ RootToken, Newlines, TokenIndent, TokenIndent, /*AlignedTo=*/nullptr,
+ /*InPPDirective=*/false,
+ /*IndentedFromColumn=*/0,
+ /*CreateReplacement=*/!Line.InMacroExpansion);
return;
}
@@ -1787,10 +1813,10 @@ void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line,
Indent = 0;
}
- Whitespaces->replaceWhitespace(RootToken, RootToken.Newlines, Indent, Indent,
- /*AlignedTo=*/nullptr,
- Line.InPPDirective &&
- !RootToken.HasUnescapedNewline);
+ Whitespaces->replaceWhitespace(
+ RootToken, RootToken.Newlines, Indent, Indent, /*AlignedTo=*/nullptr,
+ Line.InPPDirective && !RootToken.HasUnescapedNewline,
+ /*IndentedFromColumn=*/0, /*CreateReplacement=*/!Line.InMacroExpansion);
}
unsigned
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 901335950b9fac..e5084314d45dec 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -230,19 +230,22 @@ void UnwrappedLineParser::parse() {
// replaced by the expansion.
if (!ExpandedLines.empty()) {
LLVM_DEBUG(llvm::dbgs() << "Expanded lines:\n");
- for (const auto &Line : Lines) {
+ for (auto &Line : Lines) {
if (!Line.Tokens.empty()) {
auto it = ExpandedLines.find(Line.Tokens.begin()->Tok);
if (it != ExpandedLines.end()) {
- for (const auto &Expanded : it->second) {
+ for (auto &Expanded : it->second) {
+ Expanded.InMacroExpansion = true;
LLVM_DEBUG(printDebugInfo(Expanded));
Callback.consumeUnwrappedLine(Expanded);
}
continue;
}
}
+ Line.InMacroExpansion = true;
LLVM_DEBUG(printDebugInfo(Line));
Callback.consumeUnwrappedLine(Line);
+ Line.InMacroExpansion = false;
}
Callback.finishRun();
}
@@ -4802,6 +4805,8 @@ void UnwrappedLineParser::addUnwrappedLine(LineLevel AdjustLevel) {
llvm::dbgs() << "Adding unexpanded line:\n";
printDebugInfo(Reconstructed);
});
+ if (CurrentExpandedLines.size() > 1)
+ Reconstructed.Tokens.back().Tok->MacroParent = true;
ExpandedLines[Reconstructed.Tokens.begin()->Tok] = CurrentExpandedLines;
Lines.push_back(std::move(Reconstructed));
CurrentExpandedLines.clear();
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 2e1755ad3b2c68..b636291bf45e78 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -49,6 +49,9 @@ struct UnwrappedLine {
bool InPragmaDirective = false;
/// Whether it is part of a macro body.
bool InMacroBody = false;
+ /// Whether this \c UnwrappedLine is part of the first pass of a multi-pass
+ /// macro expansion run.
+ bool InMacroExpansion = false;
/// Nesting level of unbraced body of a control statement.
unsigned UnbracedBodyLevel = 0;
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index cd2feb3a5964b2..314400a7fd45e4 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -67,19 +67,17 @@ WhitespaceManager::Change::Change(
StartOfBlockComment(nullptr), IndentationOffset(0), ConditionalsLevel(0) {
}
-void WhitespaceManager::replaceWhitespace(FormatToken &Tok, unsigned Newlines,
- unsigned Spaces,
- unsigned StartOfTokenColumn,
- const FormatToken *AlignedTo,
- bool InPPDirective,
- unsigned IndentedFromColumn) {
+void WhitespaceManager::replaceWhitespace(
+ FormatToken &Tok, unsigned Newlines, unsigned Spaces,
+ unsigned StartOfTokenColumn, const FormatToken *AlignedTo,
+ bool InPPDirective, unsigned IndentedFromColumn, bool CreateReplacement) {
if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
return;
- Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
- Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
- Spaces, StartOfTokenColumn, IndentedFromColumn,
- Newlines, "", "", AlignedTo,
- InPPDirective && !Tok.IsFirst,
+ if (CreateReplacement)
+ Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
+ Changes.push_back(Change(Tok, CreateReplacement, Tok.WhitespaceRange, Spaces,
+ StartOfTokenColumn, IndentedFromColumn, Newlines, "",
+ "", AlignedTo, InPPDirective && !Tok.IsFirst,
/*IsInsideToken=*/false));
}
diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h
index 66a064ae81dbfa..6174e4961d7639 100644
--- a/clang/lib/Format/WhitespaceManager.h
+++ b/clang/lib/Format/WhitespaceManager.h
@@ -58,7 +58,8 @@ class WhitespaceManager {
unsigned StartOfTokenColumn,
const FormatToken *AlignedTo = nullptr,
bool InPPDirective = false,
- unsigned IndentedFromColumn = 0);
+ unsigned IndentedFromColumn = 0,
+ bool CreateReplacement = true);
/// Adds information about an unchangeable token's whitespace.
///
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index ca8a5180c32d54..7bc9cc98f4e248 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -230,12 +230,67 @@ a))",
TEST_F(FormatTestMacroExpansion, KeepParensWhenExpandingObjectLikeMacros) {
FormatStyle Style = getLLVMStyle();
Style.Macros.push_back("FN=class C { int f");
- verifyFormat("void f() { FN(a *b);\n"
+ verifyFormat("void f() {\n"
+ " FN(a *b);\n"
" };\n"
"}",
Style);
}
+TEST_F(FormatTestMacroExpansion, ShortFunctionsAndBlocksWithMacroExpansion) {
+ FormatStyle Style = getLLVMStyleWithColumns(35);
+ Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
+ Style.Macros = {
+ "ID(x)=x",
+ "VERY_LONG_MACRO_NAME(x)=x",
+ "EXPANDS_LONG(x)=very_long_function_call(x)",
+ "IF_ERROR_RETURN(x)=if (x) return x",
+ "STMTS=f(); g();",
+ "STMTS_NO_SEMI=f(); g()",
+ };
+
+ verifyFormat("void f() { ID(a *b); }", Style);
+ verifyFormat("void f() { int x = ID(a * b); }", Style);
+ verifyFormat("void f() {\n"
+ " VERY_LONG_MACRO_NAME(a *b);\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " int x = VERY_LONG_MACRO_NAME(x);\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " EXPANDS_LONG(a * b) +\n"
+ " c;\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " IF_ERROR_RETURN(x);\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " IF_ERROR_RETURN(a * b);\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " STMTS\n"
+ "}",
+ Style);
+ verifyFormat("void f() {\n"
+ " STMTS_NO_SEMI;\n"
+ "}",
+ Style);
+
+ verifyFormat("void f() {\n"
+ " if (a) ID(b *c);\n"
+ " if (a)\n"
+ " VERY_LONG_MACRO_NAME(b *c);\n"
+ " if (a)\n"
+ " IF_ERROR_RETURN(b * c);\n"
+ "}",
+ Style);
+}
+
TEST_F(FormatTestMacroExpansion, DoesNotExpandFunctionLikeMacrosWithoutParens) {
FormatStyle Style = getLLVMStyle();
Style.Macros.push_back("CLASS()=class C {");
More information about the cfe-commits
mailing list