[clang] 3743f56 - [clang-format] Fix macro expansion crashes and raw string indentation (#233758)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 11:59:17 PDT 2026
Author: r4nt
Date: 2026-10-21T20:59:11+02:00
New Revision: 3743f56d187182e0dc5a6c4eb373dd9d3e7251a6
URL: https://github.com/llvm/llvm-project/commit/3743f56d187182e0dc5a6c4eb373dd9d3e7251a6
DIFF: https://github.com/llvm/llvm-project/commit/3743f56d187182e0dc5a6c4eb373dd9d3e7251a6.diff
LOG: [clang-format] Fix macro expansion crashes and raw string indentation (#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 #233300
Added:
Modified:
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/TokenAnalyzer.cpp
clang/lib/Format/TokenAnalyzer.h
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTestMacroExpansion.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index b69eca38a8ef9..98232bb16cc04 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -2558,7 +2558,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 6ba3b05312607..73e567ce11538 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 ef559099d325d..40f95684e10dc 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 304d8649f2e81..2be02c8eea36a 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1246,13 +1246,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 f0531d2c8a9df..1b00e9015ea6e 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -815,7 +815,7 @@ FormatToken *UnwrappedLineParser::parseBlock(
!parsingPPDirective() ? PreprocessorDirectives.size() : 0;
addUnwrappedLine();
size_t OpeningLineIndex =
- CurrentLines->empty()
+ CurrentLines->empty() || !CurrentExpandedLines.empty()
? (UnwrappedLine::kInvalidIndex)
: (CurrentLines->size() - 1 - NbPreprocessorDirectives);
@@ -876,6 +876,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))
@@ -933,7 +935,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
@@ -4071,9 +4074,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;
@@ -4083,8 +4087,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 7bc9cc98f4e24..31fce2dbd8311 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -494,6 +494,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