[ClangFormat] Fix indent in child lines within a macro argument. #82523
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When reconstructing lines from a macro expansion, make sure that lines at different levels in the expanded code get indented correctly as part of the macro argument.
|
@llvm/pr-subscribers-clang-format Author: None (r4nt) ChangesWhen reconstructing lines from a macro expansion, make sure that lines at different levels in the expanded code get indented correctly as part of the macro argument. Full diff: https://github.com/llvm/llvm-project/pull/82523.diff 6 Files Affected:
diff --git a/clang/lib/Format/MacroCallReconstructor.cpp b/clang/lib/Format/MacroCallReconstructor.cpp
index cbdd1683c54d1a..5277b406c81b11 100644
--- a/clang/lib/Format/MacroCallReconstructor.cpp
+++ b/clang/lib/Format/MacroCallReconstructor.cpp
@@ -19,6 +19,7 @@
#include "clang/Basic/TokenKinds.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/Debug.h"
+#include <algorithm>
#include <cassert>
#define DEBUG_TYPE "format-reconstruct"
@@ -33,7 +34,7 @@ void forEachToken(const UnwrappedLine &Line, const T &Call,
FormatToken *Parent = nullptr) {
bool First = true;
for (const auto &N : Line.Tokens) {
- Call(N.Tok, Parent, First);
+ Call(N.Tok, Parent, First, Line.Level);
First = false;
for (const auto &Child : N.Children)
forEachToken(Child, Call, N.Tok);
@@ -44,7 +45,7 @@ MacroCallReconstructor::MacroCallReconstructor(
unsigned Level,
const llvm::DenseMap<FormatToken *, std::unique_ptr<UnwrappedLine>>
&ActiveExpansions)
- : Level(Level), IdToReconstructed(ActiveExpansions) {
+ : Result(Level), IdToReconstructed(ActiveExpansions) {
Result.Tokens.push_back(std::make_unique<LineNode>());
ActiveReconstructedLines.push_back(&Result);
}
@@ -52,9 +53,8 @@ MacroCallReconstructor::MacroCallReconstructor(
void MacroCallReconstructor::addLine(const UnwrappedLine &Line) {
assert(State != Finalized);
LLVM_DEBUG(llvm::dbgs() << "MCR: new line...\n");
- forEachToken(Line, [&](FormatToken *Token, FormatToken *Parent, bool First) {
- add(Token, Parent, First);
- });
+ forEachToken(Line, [&](FormatToken *Token, FormatToken *Parent, bool First,
+ unsigned Level) { add(Token, Parent, First, Level); });
assert(InProgress || finished());
}
@@ -62,8 +62,8 @@ UnwrappedLine MacroCallReconstructor::takeResult() && {
finalize();
assert(Result.Tokens.size() == 1 &&
Result.Tokens.front()->Children.size() == 1);
- UnwrappedLine Final =
- createUnwrappedLine(*Result.Tokens.front()->Children.front(), Level);
+ UnwrappedLine Final = createUnwrappedLine(
+ *Result.Tokens.front()->Children.front(), Result.Level);
assert(!Final.Tokens.empty());
return Final;
}
@@ -72,7 +72,8 @@ UnwrappedLine MacroCallReconstructor::takeResult() && {
// ExpandedParent in the incoming unwrapped line. \p First specifies whether it
// is the first token in a given unwrapped line.
void MacroCallReconstructor::add(FormatToken *Token,
- FormatToken *ExpandedParent, bool First) {
+ FormatToken *ExpandedParent, bool First,
+ unsigned Level) {
LLVM_DEBUG(
llvm::dbgs() << "MCR: Token: " << Token->TokenText << ", Parent: "
<< (ExpandedParent ? ExpandedParent->TokenText : "<null>")
@@ -102,7 +103,7 @@ void MacroCallReconstructor::add(FormatToken *Token,
First = true;
}
- prepareParent(ExpandedParent, First);
+ prepareParent(ExpandedParent, First, Level);
if (Token->MacroCtx) {
// If this token was generated by a macro call, add the reconstructed
@@ -129,7 +130,7 @@ void MacroCallReconstructor::add(FormatToken *Token,
// is the parent of ActiveReconstructedLines.back() in the reconstructed
// unwrapped line.
void MacroCallReconstructor::prepareParent(FormatToken *ExpandedParent,
- bool NewLine) {
+ bool NewLine, unsigned Level) {
LLVM_DEBUG({
llvm::dbgs() << "ParentMap:\n";
debugParentMap();
@@ -172,7 +173,7 @@ void MacroCallReconstructor::prepareParent(FormatToken *ExpandedParent,
}
assert(!ActiveReconstructedLines.empty());
ActiveReconstructedLines.back()->Tokens.back()->Children.push_back(
- std::make_unique<ReconstructedLine>());
+ std::make_unique<ReconstructedLine>(Level));
ActiveReconstructedLines.push_back(
&*ActiveReconstructedLines.back()->Tokens.back()->Children.back());
} else if (parentLine().Tokens.back()->Tok != Parent) {
@@ -424,7 +425,8 @@ bool MacroCallReconstructor::processNextReconstructed() {
SpelledParentToReconstructedParent[MacroCallStructure.back()
.ParentLastToken] = Token;
appendToken(Token);
- prepareParent(Token, /*NewLine=*/true);
+ prepareParent(Token, /*NewLine=*/true,
+ MacroCallStructure.back().Line->Level);
Token->MacroParent = true;
return false;
}
@@ -435,7 +437,8 @@ bool MacroCallReconstructor::processNextReconstructed() {
[MacroCallStructure.back().Line->Tokens.back()->Tok] = Token;
Token->MacroParent = true;
appendToken(Token, MacroCallStructure.back().Line);
- prepareParent(Token, /*NewLine=*/true);
+ prepareParent(Token, /*NewLine=*/true,
+ MacroCallStructure.back().Line->Level);
return true;
}
if (Token->is(tok::r_paren)) {
@@ -509,16 +512,36 @@ MacroCallReconstructor::createUnwrappedLine(const ReconstructedLine &Line,
for (const auto &N : Line.Tokens) {
Result.Tokens.push_back(N->Tok);
UnwrappedLineNode &Current = Result.Tokens.back();
- for (const auto &Child : N->Children) {
- if (Child->Tokens.empty())
- continue;
- Current.Children.push_back(createUnwrappedLine(*Child, Level + 1));
- }
- if (Current.Children.size() == 1 &&
- Current.Tok->isOneOf(tok::l_paren, tok::comma)) {
- Result.Tokens.splice(Result.Tokens.end(),
- Current.Children.front().Tokens);
- Current.Children.clear();
+ auto NumChildren =
+ std::count_if(N->Children.begin(), N->Children.end(),
+ [](const auto &Child) { return !Child->Tokens.empty(); });
+ if (NumChildren == 1 && Current.Tok->isOneOf(tok::l_paren, tok::comma)) {
+ // If we only have one child, and the child is due to a macro expansion
+ // (either attached to a left parenthesis or comma), merge the child into
+ // the current line to prevent forced breaks for macro arguments.
+ auto *Child = std::find_if(
+ N->Children.begin(), N->Children.end(),
+ [](const auto &Child) { return !Child->Tokens.empty(); });
+ auto Line = createUnwrappedLine(**Child, Level);
+ Result.Tokens.splice(Result.Tokens.end(), Line.Tokens);
+ } else if (NumChildren > 0) {
+ // When there are multiple children with different indent, make sure that
+ // we indent them:
+ // 1. One level below the current line's level.
+ // 2. At the correct level relative to each other.
+ unsigned MinChildLevel =
+ std::min_element(N->Children.begin(), N->Children.end(),
+ [](const auto &E1, const auto &E2) {
+ return E1->Level < E2->Level;
+ })
+ ->get()
+ ->Level;
+ for (const auto &Child : N->Children) {
+ if (Child->Tokens.empty())
+ continue;
+ Current.Children.push_back(createUnwrappedLine(
+ *Child, Level + 1 + (Child->Level - MinChildLevel)));
+ }
}
}
return Result;
diff --git a/clang/lib/Format/Macros.h b/clang/lib/Format/Macros.h
index 1964624e828ce3..d2f7fe502364cd 100644
--- a/clang/lib/Format/Macros.h
+++ b/clang/lib/Format/Macros.h
@@ -231,8 +231,9 @@ class MacroCallReconstructor {
UnwrappedLine takeResult() &&;
private:
- void add(FormatToken *Token, FormatToken *ExpandedParent, bool First);
- void prepareParent(FormatToken *ExpandedParent, bool First);
+ void add(FormatToken *Token, FormatToken *ExpandedParent, bool First,
+ unsigned Level);
+ void prepareParent(FormatToken *ExpandedParent, bool First, unsigned Level);
FormatToken *getParentInResult(FormatToken *Parent);
void reconstruct(FormatToken *Token);
void startReconstruction(FormatToken *Token);
@@ -272,6 +273,8 @@ class MacroCallReconstructor {
// FIXME: Investigate changing UnwrappedLine to a pointer type and using it
// instead of rolling our own type.
struct ReconstructedLine {
+ explicit ReconstructedLine(unsigned Level) : Level(Level) {}
+ unsigned Level;
llvm::SmallVector<std::unique_ptr<LineNode>> Tokens;
};
@@ -373,9 +376,6 @@ class MacroCallReconstructor {
// \- )
llvm::SmallVector<MacroCallState> MacroCallStructure;
- // Level the generated UnwrappedLine will be at.
- const unsigned Level;
-
// Maps from identifier of the macro call to an unwrapped line containing
// all tokens of the macro call.
const llvm::DenseMap<FormatToken *, std::unique_ptr<UnwrappedLine>>
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 8f6453a25d9d41..3a424bdcde793a 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -90,6 +90,12 @@ class ScopedDeclarationState {
} // end anonymous namespace
+std::ostream &operator<<(std::ostream &Stream, const UnwrappedLine &Line) {
+ llvm::raw_os_ostream OS(Stream);
+ printLine(OS, Line);
+ return Stream;
+}
+
class ScopedLineState {
public:
ScopedLineState(UnwrappedLineParser &Parser,
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 739298690bbd76..1403533a2d0ef6 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -420,6 +420,8 @@ struct UnwrappedLineNode {
SmallVector<UnwrappedLine, 0> Children;
};
+std::ostream &operator<<(std::ostream &Stream, const UnwrappedLine &Line);
+
} // end namespace format
} // end namespace clang
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index 653ec2a94c64da..ad5d7941bd4485 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -48,7 +48,7 @@ TEST_F(FormatTestMacroExpansion, UnexpandConfiguredMacros) {
)",
Style);
verifyIncompleteFormat("ID3({, ID(a *b),\n"
- " ;\n"
+ " ;\n"
" });",
Style);
@@ -131,9 +131,9 @@ ID(CALL(CALL(a * b)));
EXPECT_EQ(R"(
ID3(
{
- CLASS
- a *b;
- };
+ CLASS
+ a *b;
+ };
},
ID(x *y);
,
@@ -287,6 +287,21 @@ TEST_F(FormatTestMacroExpansion,
Style);
}
+TEST_F(FormatTestMacroExpansion, IndentChildrenWithinMacroCall) {
+ FormatStyle Style = getGoogleStyleWithColumns(22);
+ Style.Macros.push_back("MACRO(a, b)=a=(b)");
+ verifyFormat("void f() {\n"
+ " MACRO(a b, call([] {\n"
+ " if (expr) {\n"
+ " indent();\n"
+ " }\n"
+ " }));\n"
+ "}"
+
+ ,
+ Style);
+}
+
} // namespace
} // namespace test
} // namespace format
diff --git a/clang/unittests/Format/MacroCallReconstructorTest.cpp b/clang/unittests/Format/MacroCallReconstructorTest.cpp
index 6e6900577d165c..9df21eae70cb75 100644
--- a/clang/unittests/Format/MacroCallReconstructorTest.cpp
+++ b/clang/unittests/Format/MacroCallReconstructorTest.cpp
@@ -151,17 +151,21 @@ class MacroCallReconstructorTest : public ::testing::Test {
Lex.Allocator, Lex.IdentTable);
}
- UnwrappedLine line(llvm::ArrayRef<FormatToken *> Tokens) {
+ UnwrappedLine line(llvm::ArrayRef<FormatToken *> Tokens, unsigned Level = 0) {
UnwrappedLine Result;
+ Result.Level = Level;
for (FormatToken *Tok : Tokens)
Result.Tokens.push_back(UnwrappedLineNode(Tok));
return Result;
}
- UnwrappedLine line(llvm::StringRef Text) { return line({lex(Text)}); }
+ UnwrappedLine line(llvm::StringRef Text, unsigned Level = 0) {
+ return line({lex(Text)}, Level);
+ }
- UnwrappedLine line(llvm::ArrayRef<Chunk> Chunks) {
+ UnwrappedLine line(llvm::ArrayRef<Chunk> Chunks, unsigned Level = 0) {
UnwrappedLine Result;
+ Result.Level = Level;
for (const Chunk &Chunk : Chunks) {
Result.Tokens.insert(Result.Tokens.end(), Chunk.Tokens.begin(),
Chunk.Tokens.end());
@@ -186,6 +190,8 @@ class MacroCallReconstructorTest : public ::testing::Test {
};
bool matchesTokens(const UnwrappedLine &L1, const UnwrappedLine &L2) {
+ if (L1.Level != L2.Level)
+ return false;
if (L1.Tokens.size() != L2.Tokens.size())
return false;
for (auto L1It = L1.Tokens.begin(), L2It = L2.Tokens.begin();
@@ -288,7 +294,8 @@ TEST_F(MacroCallReconstructorTest, StatementSequence) {
matchesLine(line(
{U1.consume("SEMI"),
children({line({U2.consume("SEMI"),
- children({line(U3.consume("SEMI"))})})})})));
+ children({line(U3.consume("SEMI"), 2)})},
+ 1)})})));
}
TEST_F(MacroCallReconstructorTest, NestedBlock) {
@@ -337,9 +344,9 @@ TEST_F(MacroCallReconstructorTest, NestedBlock) {
auto Expected = line({Chunk2Start,
children({
- line(Chunk2LBrace),
- line({Chunk1, Chunk2Mid}),
- line(Chunk2RBrace),
+ line(Chunk2LBrace, 1),
+ line({Chunk1, Chunk2Mid}, 1),
+ line(Chunk2RBrace, 1),
}),
Chunk2End});
EXPECT_THAT(std::move(Unexp).takeResult(), matchesLine(Expected));
@@ -379,9 +386,11 @@ TEST_F(MacroCallReconstructorTest, NestedChildBlocks) {
Unexp.addLine(
line({E.consume("f([] {"),
children({line({E.consume("f([] {"),
- children({line(E.consume("return a * b;"))}),
- E.consume("})")})}),
- E.consume("})")}));
+ children({line(E.consume("return a * b;"), 3)}),
+ E.consume("})")},
+ 2)}),
+ E.consume("})")},
+ 1));
Unexp.addLine(line(E.consume("}")));
EXPECT_TRUE(Unexp.finished());
@@ -407,13 +416,15 @@ TEST_F(MacroCallReconstructorTest, NestedChildBlocks) {
auto Expected = line({
Chunk3Start,
children({
- line(Chunk3LBrace),
- line({
- Chunk2Start,
- Chunk1,
- Chunk2End,
- }),
- line(Chunk3RBrace),
+ line(Chunk3LBrace, 1),
+ line(
+ {
+ Chunk2Start,
+ Chunk1,
+ Chunk2End,
+ },
+ 2),
+ line(Chunk3RBrace, 1),
}),
Chunk3End,
});
@@ -469,8 +480,8 @@ TEST_F(MacroCallReconstructorTest, MultipleToplevelUnwrappedLines) {
auto Expected = line({
U.consume("ID("),
children({
- line(U.consume("x;")),
- line(U.consume("x")),
+ line(U.consume("x;"), 1),
+ line(U.consume("x"), 1),
}),
U.consume(", y)"),
});
@@ -524,9 +535,9 @@ TEST_F(MacroCallReconstructorTest, NestedCallsMultipleLines) {
auto Expected = line({
Chunk2Start,
children({
- line({Chunk2LBrace}),
- line({Chunk1, Chunk2Semi}),
- line({Chunk2RBrace}),
+ line({Chunk2LBrace}, 1),
+ line({Chunk1, Chunk2Semi}, 1),
+ line({Chunk2RBrace}, 1),
}),
Chunk2End,
});
@@ -556,15 +567,17 @@ TEST_F(MacroCallReconstructorTest, ParentOutsideMacroCall) {
auto Expected = line({
Prefix,
children({
- line({
- U.consume("ID("),
- children({
- line(U.consume("x;")),
- line(U.consume("y;")),
- line(U.consume("z;")),
- }),
- U.consume(")"),
- }),
+ line(
+ {
+ U.consume("ID("),
+ children({
+ line(U.consume("x;"), 2),
+ line(U.consume("y;"), 2),
+ line(U.consume("z;"), 2),
+ }),
+ U.consume(")"),
+ },
+ 1),
}),
Postfix,
});
@@ -590,7 +603,7 @@ TEST_F(MacroCallReconstructorTest, ChildrenSplitAcrossArguments) {
Matcher U(Call, Lex);
auto Expected = line({
U.consume("CALL({"),
- children(line(U.consume("a;"))),
+ children(line(U.consume("a;"), 1)),
U.consume(", b; })"),
});
EXPECT_THAT(std::move(Unexp).takeResult(), matchesLine(Expected));
@@ -620,16 +633,20 @@ TEST_F(MacroCallReconstructorTest, ChildrenAfterMacroCall) {
Matcher U(Call, Lex);
auto Expected = line({
U.consume("CALL({"),
- children(line(U.consume("a"))),
+ children(line(U.consume("a"), 1)),
U.consume(", b)"),
Semi,
- children(line({
- SecondLine,
- children(line({
- ThirdLine,
- Postfix,
- })),
- })),
+ children(line(
+ {
+ SecondLine,
+ children(line(
+ {
+ ThirdLine,
+ Postfix,
+ },
+ 2)),
+ },
+ 1)),
});
EXPECT_THAT(std::move(Unexp).takeResult(), matchesLine(Expected));
}
@@ -655,7 +672,37 @@ TEST_F(MacroCallReconstructorTest, InvalidCodeSplittingBracesAcrossArgs) {
Matcher U(Call, Lex);
auto Expected = line({
Prefix,
- children({line(U.consume("M({,x,)"))}),
+ children({line(U.consume("M({,x,)"), 1)}),
+ });
+ EXPECT_THAT(std::move(Unexp).takeResult(), matchesLine(Expected));
+}
+
+TEST_F(MacroCallReconstructorTest, IndentLevelInExpandedCode) {
+ auto Macros = createExpander({"ID(a)=a"});
+ Expansion Exp(Lex, *Macros);
+ TokenList Call = Exp.expand("ID", {std::string("[] { { x; } }")});
+
+ MacroCallReconstructor Unexp(0, Exp.getUnexpanded());
+ Matcher E(Exp.getTokens(), Lex);
+ Unexp.addLine(line({
+ E.consume("[] {"),
+ children({
+ line(E.consume("{"), 1),
+ line(E.consume("x;"), 2),
+ line(E.consume("}"), 1),
+ }),
+ E.consume("}"),
+ }));
+ EXPECT_TRUE(Unexp.finished());
+ Matcher U(Call, Lex);
+ auto Expected = line({
+ U.consume("ID([] {"),
+ children({
+ line(U.consume("{"), 1),
+ line(U.consume("x;"), 2),
+ line(U.consume("}"), 1),
+ }),
+ U.consume("})"),
});
EXPECT_THAT(std::move(Unexp).takeResult(), matchesLine(Expected));
}
|
owenca
reviewed
Feb 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When reconstructing lines from a macro expansion, make sure that lines at different levels in the expanded code get indented correctly as part of the macro argument.