https://github.com/r4nt created https://github.com/llvm/llvm-project/pull/233758
Fix three issues with clang-format's macro expansion (FormatStyle::Macros):
1. Guard MatchingOpeningBlockLineIndex / MatchingClosingBlockLineIndex during
macro expansion: When a macro expands to a multi-line block (such as
DECL_STRUCT(Name)=struct Name {}), UnwrappedLineParser::addUnwrappedLine pushes
the expanded lines into CurrentExpandedLines rather than CurrentLines. However,
parseBlock() and parseEnum() still computed OpeningLineIndex from
CurrentLines->size() - 1, recording an unrelated or out-of-bounds line index in
MatchingOpeningBlockLineIndex and MatchingClosingBlockLineIndex and crashing
DefinitionBlockSeparator / NamespaceEndCommentsFixer. Only record block line
indices when CurrentExpandedLines is empty and the closing line is not part of
a macro expansion.
2. Annotate and analyze each run immediately in TokenAnalyzer::finishRun():
FormatToken objects in AllTokens are shared across preprocessor branch runs in
UnwrappedLineParser::parse(), and UnwrappedLineParser::reset() resets
Token->MacroCtx at the start of each branch. Previously, TokenAnalyzer stored a
nested vector of runs (SmallVector<SmallVector<UnwrappedLine, 16>, 2>
UnwrappedLines), appending a new inner vector on each finishRun() while
Parser.parse() parsed all #if / #else branches, and only iterated over
UnwrappedLines to annotate and analyze after Parser.parse() returned. Because
later #if / #else branches reset Token->MacroCtx and mutated shared
FormatTokens during parsing, earlier runs had their token state clobbered
before they were ever annotated or analyzed. Instead, keep only the current
run's lines in a flat SmallVector<UnwrappedLine, 16> UnwrappedLines, invoke
OnFinishRun() inside finishRun() to annotate and analyze the current run
immediately before the parser moves to the next run/branch, and then clear
UnwrappedLines.
3. Indent multi-line raw string literals in unexpanded macro arguments: When a
multi-line raw string literal (e.g. R"pb(...)pb") is passed as a macro
argument, ContinuationIndenter::handleEndOfLine previously reformatted the raw
string literal during the expanded pass (MR_ExpandedArg) using the expanded
statement's indentation, and UnwrappedLineFormatter did not update
ParenState::NestedBlockIndent after formatting a MacroParent's children in the
unexpanded pass. Skip reformatRawStringLiteral when Role == MR_ExpandedArg and
update NestedBlockIndent after formatting MacroParent children.
Fixes #78263
Fixes #183717
Fixes #233300
>From 888514e925ab6208131f0e70ce0ee964715a3fde Mon Sep 17 00:00:00 2001
From: Manuel Klimek <[email protected]>
Date: Mon, 19 Oct 2026 14:21:13 +0200
Subject: [PATCH] [clang-format] Fix macro expansion crashes and raw string
indentation
Fix three issues with clang-format's macro expansion (FormatStyle::Macros):
1. Guard MatchingOpeningBlockLineIndex / MatchingClosingBlockLineIndex
during macro expansion:
When a macro expands to a multi-line block (such as
DECL_STRUCT(Name)=struct Name {}), UnwrappedLineParser::addUnwrappedLine
pushes the expanded lines into CurrentExpandedLines rather than
CurrentLines. However, parseBlock() and parseEnum() still computed
OpeningLineIndex from CurrentLines->size() - 1, recording an unrelated
or out-of-bounds line index in MatchingOpeningBlockLineIndex and
MatchingClosingBlockLineIndex and crashing DefinitionBlockSeparator /
NamespaceEndCommentsFixer. Only record block line indices when
CurrentExpandedLines is empty and the closing line is not part of a
macro expansion.
2. Annotate and analyze each run immediately in TokenAnalyzer::finishRun():
FormatToken objects in AllTokens are shared across preprocessor branch
runs in UnwrappedLineParser::parse(), and UnwrappedLineParser::reset()
resets Token->MacroCtx at the start of each branch. Previously,
TokenAnalyzer stored a nested vector of runs
(SmallVector<SmallVector<UnwrappedLine, 16>, 2> UnwrappedLines),
appending a new inner vector on each finishRun() while Parser.parse()
parsed all #if / #else branches, and only iterated over UnwrappedLines
to annotate and analyze after Parser.parse() returned. Because later
#if / #else branches reset Token->MacroCtx and mutated shared
FormatTokens during parsing, earlier runs had their token state
clobbered before they were ever annotated or analyzed. Instead, keep
only the current run's lines in a flat SmallVector<UnwrappedLine, 16>
UnwrappedLines, invoke OnFinishRun() inside finishRun() to annotate and
analyze the current run immediately before the parser moves to the next
run/branch, and then clear UnwrappedLines.
3. Indent multi-line raw string literals in unexpanded macro arguments:
When a multi-line raw string literal (e.g. R"pb(...)pb") is passed as a
macro argument, ContinuationIndenter::handleEndOfLine previously
reformatted the raw string literal during the expanded pass
(MR_ExpandedArg) using the expanded statement's indentation, and
UnwrappedLineFormatter did not update ParenState::NestedBlockIndent
after formatting a MacroParent's children in the unexpanded pass. Skip
reformatRawStringLiteral when Role == MR_ExpandedArg and update
NestedBlockIndent after formatting MacroParent children.
Fixes #78263
Fixes #183717
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..0bd83c8d97673d 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://github.com/llvm/llvm-project/issues/78263
+ 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://github.com/llvm/llvm-project/issues/233300
+ 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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
