[clang] [clang-format] Fix macro expansion crashes and raw string indentation (PR #233758)
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/233758
>From 611804c5de0723c37f25282fae9c30f6adb40120 Mon Sep 17 00:00:00 2001
From: Manuel Klimek <klimek at google.com>
Date: Mon, 19 Oct 2026 14:21:13 +0200
Subject: [PATCH] [clang-format] Fix child line formatting and indentation in
macro expansion
Fix three issues when expanding configured macros (`Style.Macros`):
1. Avoid formatting child lines of reconstructed macro calls twice
(Fixes #78263):
`UnwrappedLineFormatter::format` formats `Line.Children` of tokens in a
line. When formatting the reconstructed unexpanded macro call line, its
arguments' child lines (such as a block inside `PACK(struct Foo { ... })` or
a raw string literal inside `ASSIGN_OR_RETURN(...)`) were already formatted
during the expanded macro pass (`MR_ExpandedArg`), or vice versa. Formatting
the macro argument's `Children` during the `MR_ExpandedArg` pass caused
duplicate whitespace replacements (crashing in `WhitespaceManager` when
`SeparateDefinitionBlocks` is enabled) and caused
`ContinuationIndenter::reformatRawStringLiteral` to use the expanded pass's
column indentation instead of the reconstructed macro call's indentation.
Skip formatting `Children` of `MR_ExpandedArg` tokens during the expanded
pass and format `Children` of `MR_UnexpandedArg` tokens during the
reconstructed pass, passing the parent line's `DryRun` state down into
recursive `LineFormatter->format` calls.
2. Annotate and analyze each run in `TokenAnalyzer::finishRun()`
(Fixes #233300):
Previously, `UnwrappedLineParser::parse()` ran all `#if`/`#else` branch
passes before `TokenAnalyzer::process()` annotated any run. Because
`FormatToken` objects are shared across branches, macro reconstruction in
one branch left `MacroCtx` set on shared tokens, causing a subsequent branch
to re-reconstruct tokens with finalized types (`FormatToken::setType`
assertion failure). Instead, annotate and analyze each run inside
`TokenAnalyzer::finishRun()` as soon as the parser finishes that branch, and
reset `Token->MacroCtx` in `UnwrappedLineParser::reset()` before parsing the
next branch.
3. Guard against empty reconstructed macro lines on syntax errors:
In `UnwrappedLineParser::addUnwrappedLine()`, check `!Line->Tokens.empty()`
after `Reconstruct->takeResult()` so malformed macro calls that reconstruct
to an empty line do not dereference an empty token list.
Fixes #78263
Fixes #233300
---
clang/lib/Format/ContinuationIndenter.cpp | 3 +-
clang/lib/Format/TokenAnalyzer.cpp | 36 +++---
clang/lib/Format/TokenAnalyzer.h | 3 +-
clang/lib/Format/UnwrappedLineFormatter.cpp | 4 +-
clang/lib/Format/UnwrappedLineParser.cpp | 18 ++-
.../Format/FormatTestMacroExpansion.cpp | 113 ++++++++++++++++++
6 files changed, 153 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index bb8179cbc4f95f..fd950201578475 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -2557,7 +2557,8 @@ unsigned ContinuationIndenter::handleEndOfLine(const FormatToken &Current,
// Compute the raw string style to use in case this is a raw string literal
// that can be reformatted.
auto RawStringStyle = getRawStringStyle(Current, State);
- if (RawStringStyle && !Current.Finalized) {
+ if (RawStringStyle && !Current.Finalized &&
+ (!Current.MacroCtx || Current.MacroCtx->Role != MR_ExpandedArg)) {
Penalty = reformatRawStringLiteral(Current, State, *RawStringStyle, DryRun,
Newline);
} else if (Current.IsMultiline && Current.isNot(TT_BlockComment)) {
diff --git a/clang/lib/Format/TokenAnalyzer.cpp b/clang/lib/Format/TokenAnalyzer.cpp
index 6ba3b053126075..73e567ce115383 100644
--- a/clang/lib/Format/TokenAnalyzer.cpp
+++ b/clang/lib/Format/TokenAnalyzer.cpp
@@ -83,7 +83,6 @@ Environment::Environment(StringRef Code, StringRef FileName,
TokenAnalyzer::TokenAnalyzer(const Environment &Env, const FormatStyle &Style)
: Style(Style), LangOpts(getFormattingLangOpts(Style)), Env(Env),
AffectedRangeMgr(Env.getSourceManager(), Env.getCharRanges()),
- UnwrappedLines(1),
Encoding(encoding::detectEncoding(
Env.getSourceManager().getBufferData(Env.getFileID()))) {
LLVM_DEBUG(
@@ -104,20 +103,19 @@ TokenAnalyzer::process(bool SkipAnnotation) {
IdentTable);
ArrayRef<FormatToken *> Toks(Lex.lex());
SmallVector<FormatToken *, 10> Tokens(Toks);
- UnwrappedLineParser Parser(Env.getSourceManager(), Style, Lex.getKeywords(),
- Env.getFirstStartColumn(), Tokens, *this,
- Allocator, IdentTable);
- Parser.parse();
- assert(UnwrappedLines.back().empty());
+
unsigned Penalty = 0;
- for (unsigned Run = 0, RunE = UnwrappedLines.size(); Run + 1 != RunE; ++Run) {
- const auto &Lines = UnwrappedLines[Run];
+ unsigned Run = 0;
+ bool HasError = false;
+ OnFinishRun = [&]() {
+ if (HasError)
+ return;
LLVM_DEBUG(llvm::dbgs() << "Run " << Run << "...\n");
SmallVector<AnnotatedLine *, 16> AnnotatedLines;
- AnnotatedLines.reserve(Lines.size());
+ AnnotatedLines.reserve(UnwrappedLines.size());
TokenAnnotator Annotator(Style, Lex.getKeywords());
- for (const UnwrappedLine &Line : Lines) {
+ for (const UnwrappedLine &Line : UnwrappedLines) {
AnnotatedLines.push_back(new AnnotatedLine(Line));
if (!SkipAnnotation)
Annotator.annotate(*AnnotatedLines.back());
@@ -133,6 +131,7 @@ TokenAnalyzer::process(bool SkipAnnotation) {
});
for (AnnotatedLine *Line : AnnotatedLines)
delete Line;
+ ++Run;
Penalty += RunResult.second;
for (const auto &R : RunResult.first) {
@@ -141,20 +140,27 @@ TokenAnalyzer::process(bool SkipAnnotation) {
// Replacements to indicate failure.
if (Err) {
llvm::errs() << llvm::toString(std::move(Err)) << "\n";
- return {tooling::Replacements(), 0};
+ HasError = true;
+ return;
}
}
- }
+ };
+ UnwrappedLineParser Parser(Env.getSourceManager(), Style, Lex.getKeywords(),
+ Env.getFirstStartColumn(), Tokens, *this,
+ Allocator, IdentTable);
+ Parser.parse();
+ if (HasError)
+ return {tooling::Replacements(), 0};
return {Result, Penalty};
}
void TokenAnalyzer::consumeUnwrappedLine(const UnwrappedLine &TheLine) {
- assert(!UnwrappedLines.empty());
- UnwrappedLines.back().push_back(TheLine);
+ UnwrappedLines.push_back(TheLine);
}
void TokenAnalyzer::finishRun() {
- UnwrappedLines.push_back(SmallVector<UnwrappedLine, 16>());
+ OnFinishRun();
+ UnwrappedLines.clear();
}
} // end namespace format
diff --git a/clang/lib/Format/TokenAnalyzer.h b/clang/lib/Format/TokenAnalyzer.h
index ef559099d325d2..40f95684e10dc0 100644
--- a/clang/lib/Format/TokenAnalyzer.h
+++ b/clang/lib/Format/TokenAnalyzer.h
@@ -97,7 +97,8 @@ class TokenAnalyzer : public UnwrappedLineConsumer {
const Environment &Env;
// AffectedRangeMgr stores ranges to be fixed.
AffectedRangeManager AffectedRangeMgr;
- SmallVector<SmallVector<UnwrappedLine, 16>, 2> UnwrappedLines;
+ SmallVector<UnwrappedLine, 16> UnwrappedLines;
+ std::function<void()> OnFinishRun;
encoding::Encoding Encoding;
};
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index e42a2083f7268a..81131669645ccc 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1223,13 +1223,15 @@ class LineFormatter {
}
if (NewLine || Previous.MacroParent) {
- const ParenState &P = State.Stack.back();
+ ParenState &P = State.Stack.back();
int AdditionalIndent =
P.Indent.Total - Previous.Children[0]->Level * Style.IndentWidth;
Penalty +=
BlockFormatter->format(Previous.Children, DryRun, AdditionalIndent,
/*FixBadIndentation=*/true);
+ if (Previous.MacroParent)
+ P.NestedBlockIndent = P.Indent.Total;
return true;
}
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 901335950b9fac..765f495a00c827 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -812,7 +812,7 @@ FormatToken *UnwrappedLineParser::parseBlock(
!parsingPPDirective() ? PreprocessorDirectives.size() : 0;
addUnwrappedLine();
size_t OpeningLineIndex =
- CurrentLines->empty()
+ CurrentLines->empty() || !CurrentExpandedLines.empty()
? (UnwrappedLine::kInvalidIndex)
: (CurrentLines->size() - 1 - NbPreprocessorDirectives);
@@ -873,6 +873,8 @@ FormatToken *UnwrappedLineParser::parseBlock(
if (Previous->is(tok::r_brace) && !Previous->Optional)
return false;
}
+ if (OpeningLineIndex == UnwrappedLine::kInvalidIndex)
+ return false;
assert(!CurrentLines->empty());
auto &LastLine = CurrentLines->back();
if (LastLine.Level == InitialLevel + 1 && !mightFitOnOneLine(LastLine))
@@ -930,7 +932,8 @@ FormatToken *UnwrappedLineParser::parseBlock(
if (MunchSemi && FormatTok->is(tok::semi))
nextToken();
- if (PPStartHash == PPEndHash) {
+ if (PPStartHash == PPEndHash && CurrentExpandedLines.empty() &&
+ !containsExpansion(*Line)) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
// Update the opening line to add the forward reference as well
@@ -4016,9 +4019,10 @@ bool UnwrappedLineParser::parseEnum() {
if (!ManageWhitesmithsBraces)
++Line->Level;
}
- const auto OpeningLineIndex = CurrentLines->empty()
- ? UnwrappedLine::kInvalidIndex
- : CurrentLines->size() - 1;
+ const auto OpeningLineIndex =
+ CurrentLines->empty() || !CurrentExpandedLines.empty()
+ ? UnwrappedLine::kInvalidIndex
+ : CurrentLines->size() - 1;
bool HasError = !parseBracedList(/*IsAngleBracket=*/false, /*IsEnum=*/true);
if (!Style.AllowShortEnumsOnASingleLine && !ManageWhitesmithsBraces)
--Line->Level;
@@ -4028,8 +4032,10 @@ bool UnwrappedLineParser::parseEnum() {
addUnwrappedLine();
}
setPreviousRBraceType(TT_EnumRBrace);
- if (ManageWhitesmithsBraces)
+ if (ManageWhitesmithsBraces && CurrentExpandedLines.empty() &&
+ !containsExpansion(*Line)) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
+ }
return true;
// There is no addUnwrappedLine() here so that we fall through to parsing a
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index ca8a5180c32d54..4f35b19b5858f6 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -439,6 +439,119 @@ TEST_F(FormatTestMacroExpansion, TokensAfterMacroCallAreNotPartOfCall) {
Style);
}
+TEST_F(FormatTestMacroExpansion,
+ SeparateDefinitionBlocksWithMacroExpandedStruct) {
+ // Reproduces https://llvm.org/PR78263
+ FormatStyle Style = getLLVMStyle();
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterStruct = true;
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ Style.Macros.push_back("PACK(x)=x");
+
+ verifyFormat("static bool foo;\n"
+ "\n"
+ "//! \\brief Blah\n"
+ "PACK(\n"
+ " struct Foo\n"
+ " {\n"
+ " uint16_t a;\n"
+ " });\n"
+ "\n"
+ "PACK(\n"
+ " struct Bar\n"
+ " {\n"
+ " uint32_t a;\n"
+ " });\n"
+ "\n"
+ "PACK(\n"
+ " struct Baz\n"
+ " {\n"
+ " uint16_t a;\n"
+ " });",
+ "static bool foo;\n"
+ "\n"
+ "//! \\brief Blah\n"
+ "PACK(struct Foo\n"
+ "{\n"
+ " uint16_t a;\n"
+ "});\n"
+ "\n"
+ "PACK(struct Bar\n"
+ "{\n"
+ " uint32_t a;\n"
+ "});\n"
+ "\n"
+ "PACK(struct Baz\n"
+ "{\n"
+ " uint16_t a;\n"
+ "});",
+ Style);
+}
+
+TEST_F(FormatTestMacroExpansion, MacroExpansionWithIfElseBranches) {
+ // Reproduces https://llvm.org/PR233300
+ FormatStyle Style = getLLVMStyle();
+ Style.Macros.push_back("M(x)=x");
+
+ verifyFormat("#if A\n"
+ "auto l = []\n"
+ "#else\n"
+ "if\n"
+ "#endif\n"
+ " M((int a) {});",
+ Style);
+
+ Style.Macros = {"M(a,b,c)=a b(c)"};
+ verifyNoCrash("({\n"
+ "#if A\n"
+ "{[]\n"
+ "#else\n"
+ "if\n"
+ "#endif\n"
+ "M(, (int a){int b, ))",
+ Style);
+}
+
+TEST_F(FormatTestMacroExpansion, RawStringLiteralInMacroArg) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+ Style.Macros.push_back("ASSIGN_OR_RETURN(a, b)=a = (b)");
+
+ StringRef Code1 =
+ "void f() {\n"
+ " ASSIGN_OR_RETURN(FlowConfig flow_config,\n"
+ " ParseTextProto<FlowConfig>(Substitute(\n"
+ " R\"pb(\n"
+ " key: value\n"
+ " )pb\",\n"
+ " arg1, arg2, arg3, arg4)));\n"
+ "}";
+ verifyFormat(Code1, Code1, Style);
+
+ StringRef Code2 =
+ "void f() {\n"
+ " ASSIGN_OR_RETURN(\n"
+ " FlowConfig flow_config,\n"
+ " "
+ "proto2::contrib::parse_proto::ParseTextProto<FlowConfig>(absl::"
+ "Substitute(\n"
+ " R\"pb(\n"
+ " key: value\n"
+ " )pb\",\n"
+ " kPreprocessingFlowConfig, kAudioConfig, kPostConfig)));\n"
+ "}";
+ verifyFormat(Code2, Code2, Style);
+
+ StringRef Code3 =
+ "void f() {\n"
+ " ASSIGN_OR_RETURN(\n"
+ " // comment\n"
+ " TestCase test_case, ParseTextProto<TestCase>(R\"pb(\n"
+ " name: \"foo\"\n"
+ " )pb\"));\n"
+ "}";
+ verifyFormat(Code3, Code3, Style);
+}
+
} // namespace
} // namespace test
} // namespace format
More information about the cfe-commits
mailing list