enyquist updated this revision to Diff 93092.
enyquist added a comment.
Option implemented completely in WhitespaceManager.cpp, so no overhead if the
option isn't used. Also some minor style fixes.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,78 @@
verifyFormat("a or_eq 8;", Spaces);
}
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveAssignments = true;
+ Alignment.AlignConsecutiveDeclarations = true;
+ Alignment.AlignConsecutiveMacros = false;
+
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define ffff(x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define ffff(x, y) (x - y)",
+ Alignment);
+
+ Alignment.AlignConsecutiveMacros = true;
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)",
+ Alignment);
+
+ verifyFormat("#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define ffff(x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define foo(x, y) (x + y)\n"
+ "#define bar (5, 6)(2 + 2)",
+ Alignment);
+
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)\n"
+ "#define f(x) (x * x)\n"
+ "#define fff(x, y, z) (x * y + z)\n"
+ "#define ffff(x, y) (x - y)",
+ Alignment);
+
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
+ " auto ii = 0;\n"
+ " float j = 0;\n"
+ " return 0;\n"
+ "};\n"
+ "int i = 0;\n"
+ "float i2 = 0;\n"
+ "auto v = type{\n"
+ " i = 1, //\n"
+ " (i = 2), //\n"
+ " i = 3 //\n"
+ "};",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignments) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveAssignments = false;
verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -7660,7 +7730,10 @@
"int j = 2;",
Alignment);
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto i = 0;\n"
" return 0;\n"
"};\n"
@@ -7702,6 +7775,7 @@
TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
+ Alignment.AlignConsecutiveMacros = true;
Alignment.AlignConsecutiveDeclarations = false;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -7933,7 +8007,10 @@
Alignment);
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
+ "#define CCC (6)\n"
+ "auto lambda = []() {\n"
" auto ii = 0;\n"
" float j = 0;\n"
" return 0;\n"
@@ -8659,6 +8736,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignOperands);
CHECK_PARSE_BOOL(AlignTrailingComments);
+ CHECK_PARSE_BOOL(AlignConsecutiveMacros);
CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
/// \c EscapedNewlineColumn for the first tokens or token parts in a line.
void calculateLineBreakInformation();
+ /// \brief Align consecutive C/C++ preprocessor macros over all \c Changes.
+ void alignConsecutiveMacros();
+
/// \brief Align consecutive assignments over all \c Changes.
void alignConsecutiveAssignments();
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -87,6 +87,7 @@
std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr));
calculateLineBreakInformation();
+ alignConsecutiveMacros();
alignConsecutiveDeclarations();
alignConsecutiveAssignments();
alignTrailingComments();
@@ -189,7 +190,8 @@
template <typename F>
static void
AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
- SmallVector<WhitespaceManager::Change, 16> &Changes) {
+ SmallVector<WhitespaceManager::Change, 16> &Changes,
+ bool ConsiderScope) {
bool FoundMatchOnLine = false;
int Shift = 0;
@@ -207,34 +209,37 @@
SmallVector<unsigned, 16> ScopeStack;
for (unsigned i = Start; i != End; ++i) {
- if (ScopeStack.size() != 0 &&
+ if (ConsiderScope && ScopeStack.size() != 0 &&
Changes[i].nestingAndIndentLevel() <
Changes[ScopeStack.back()].nestingAndIndentLevel())
ScopeStack.pop_back();
- if (i != Start && Changes[i].nestingAndIndentLevel() >
- Changes[i - 1].nestingAndIndentLevel())
+ if (ConsiderScope && i != Start &&
+ Changes[i].nestingAndIndentLevel() >
+ Changes[i - 1].nestingAndIndentLevel())
ScopeStack.push_back(i);
bool InsideNestedScope = ScopeStack.size() != 0;
- if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
+ if (Changes[i].NewlinesBefore > 0 &&
+ (!InsideNestedScope || !ConsiderScope)) {
Shift = 0;
FoundMatchOnLine = false;
}
// If this is the first matching token to be aligned, remember by how many
// spaces it has to be shifted, so the rest of the changes on the line are
// shifted by the same amount
- if (!FoundMatchOnLine && !InsideNestedScope && Matches(Changes[i])) {
+ if (!FoundMatchOnLine && (!ConsiderScope || !InsideNestedScope) &&
+ Matches(Changes[i])) {
FoundMatchOnLine = true;
Shift = Column - Changes[i].StartOfTokenColumn;
Changes[i].Spaces += Shift;
}
// This is for function parameters that are split across multiple lines,
// as mentioned in the ScopeStack comment.
- if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
+ if (ConsiderScope && InsideNestedScope && Changes[i].NewlinesBefore > 0) {
unsigned ScopeStart = ScopeStack.back();
if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
(ScopeStart > Start + 1 &&
@@ -279,6 +284,7 @@
template <typename F>
static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes,
+ bool ConsiderScope, bool ConsiderCommas,
unsigned StartAt) {
unsigned MinColumn = 0;
unsigned MaxColumn = UINT_MAX;
@@ -293,9 +299,9 @@
? Changes[StartAt].nestingAndIndentLevel()
: std::pair<unsigned, unsigned>(0, 0);
- // Keep track of the number of commas before the matching tokens, we will only
- // align a sequence of matching tokens if they are preceded by the same number
- // of commas.
+ // Keep track of the number of commas before the matching tokens. If
+ // ConsiderCommas is true, we will only align a sequence of matching tokens
+ // if they are preceded by the same number of commas.
unsigned CommasBeforeLastMatch = 0;
unsigned CommasBeforeMatch = 0;
@@ -312,7 +318,7 @@
auto AlignCurrentSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
AlignTokenSequence(StartOfSequence, EndOfSequence, MinColumn, Matches,
- Changes);
+ Changes, ConsiderScope);
MinColumn = 0;
MaxColumn = UINT_MAX;
StartOfSequence = 0;
@@ -337,9 +343,11 @@
if (Changes[i].Tok->is(tok::comma)) {
++CommasBeforeMatch;
- } else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
+ } else if (ConsiderScope &&
+ (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel)) {
// Call AlignTokens recursively, skipping over this scope block.
- unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
+ unsigned StoppedAt = AlignTokens(Style, Matches, Changes, ConsiderScope,
+ ConsiderCommas, i);
i = StoppedAt - 1;
continue;
}
@@ -349,12 +357,12 @@
// If there is more than one matching token per line, or if the number of
// preceding commas, do not match anymore, end the sequence.
- if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch)
+ if (FoundMatchOnLine ||
+ (ConsiderCommas && (CommasBeforeMatch != CommasBeforeLastMatch)))
AlignCurrentSequence();
CommasBeforeLastMatch = CommasBeforeMatch;
FoundMatchOnLine = true;
-
if (StartOfSequence == 0)
StartOfSequence = i;
@@ -366,7 +374,7 @@
// If we are restricted by the maximum column width, end the sequence.
if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
- CommasBeforeLastMatch != CommasBeforeMatch) {
+ (ConsiderCommas && (CommasBeforeLastMatch != CommasBeforeMatch))) {
AlignCurrentSequence();
StartOfSequence = i;
}
@@ -380,6 +388,63 @@
return i;
}
+// This function heuristically determines whether 'Current' is the identifier
+// following the 'define' keyword in a simple PP macro (i.e. no parameters)
+static bool endsMacroIdentifier(const FormatToken *Current) {
+ const FormatToken *Keyword = Current->Previous;
+
+ if (!Keyword || !Current->is(tok::identifier))
+ return false;
+
+ if (!Current->Next || !Current->Next->SpacesRequiredBefore)
+ return false;
+
+ return Keyword->is(tok::pp_define);
+}
+
+// This function heuristically determines whether 'Current' is the closing
+// r_paren token for the parameter list of a function-like PP macro
+static bool endsMacroWithArgsIdentifier(const FormatToken *Current) {
+ const FormatToken *Keyword, *Param = Current->Previous;
+
+ if (!Param || !Current->is(tok::r_paren))
+ return false;
+
+ while (Param && !Param->is(tok::l_paren)) {
+ if (!Param->is(tok::identifier) && !Param->is(tok::comma))
+ return false;
+ if (!Param->Previous)
+ return false;
+
+ Param = Param->Previous;
+ }
+
+ if (!Param || Param->SpacesRequiredBefore)
+ return false;
+ if (!Param->Previous || !Param->Previous->is(tok::identifier))
+ return false;
+
+ Keyword = Param->Previous->Previous;
+ return Keyword && Keyword->is(tok::pp_define);
+}
+
+static bool endsPPIdentifier(const FormatToken *Current) {
+ return endsMacroIdentifier(Current) || endsMacroWithArgsIdentifier(Current);
+}
+
+void WhitespaceManager::alignConsecutiveMacros() {
+ if (!Style.AlignConsecutiveMacros)
+ return;
+
+ AlignTokens(Style, [&](const Change &C) {
+ if (&C == &Changes.front())
+ return false;
+
+ return endsPPIdentifier((&C - 1)->Tok);
+ },
+ Changes, false, false, /*StartAt=*/0);
+}
+
void WhitespaceManager::alignConsecutiveAssignments() {
if (!Style.AlignConsecutiveAssignments)
return;
@@ -396,7 +461,7 @@
return C.Tok->is(tok::equal);
},
- Changes, /*StartAt=*/0);
+ Changes, true, true, /*StartAt=*/0);
}
void WhitespaceManager::alignConsecutiveDeclarations() {
@@ -417,7 +482,7 @@
C.Tok->is(TT_FunctionDeclarationName) ||
C.Tok->is(tok::kw_operator);
},
- Changes, /*StartAt=*/0);
+ Changes, true, true, /*StartAt=*/0);
}
void WhitespaceManager::alignTrailingComments() {
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -243,6 +243,7 @@
IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
+ IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
IO.mapOptional("AlignConsecutiveAssignments",
Style.AlignConsecutiveAssignments);
IO.mapOptional("AlignConsecutiveDeclarations",
@@ -502,6 +503,7 @@
LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
LLVMStyle.AlignOperands = true;
LLVMStyle.AlignTrailingComments = true;
+ LLVMStyle.AlignConsecutiveMacros = false;
LLVMStyle.AlignConsecutiveAssignments = false;
LLVMStyle.AlignConsecutiveDeclarations = false;
LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -76,6 +76,19 @@
/// brackets.
BracketAlignmentStyle AlignAfterOpenBracket;
+ /// \brief If ``true``, aligns consecutive C/C++ preprocessor macros.
+ ///
+ /// This will align C/C++ preprocessor macros of consecutive lines.
+ /// Will result in formattings like
+ /// \code
+ /// #define SHORT_NAME 42
+ /// #define LONGER_NAME 0x007f
+ /// #define EVEN_LONGER_NAME (2)
+ /// #define foo(x) (x * x)
+ /// #define bar(y, z) (y + z)
+ /// \endcode
+ bool AlignConsecutiveMacros;
+
/// \brief If ``true``, aligns consecutive assignments.
///
/// This will align the assignment operators of consecutive lines. This
Index: docs/ClangFormatStyleOptions.rst
===================================================================
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -185,6 +185,20 @@
+**AlignConsecutiveMacros** (``bool``)
+ If ``true``, aligns consecutive C/C++ preprocessor macros.
+
+ This will align the C/C++ preprocessor macros of consecutive lines. This
+ will result in formattings like
+
+ .. code-block:: c++
+
+ #define SHORT_NAME 42
+ #define LONGER_NAME 0x007f
+ #define EVEN_LONGER_NAME (2)
+ #define foo(x) (x * x)
+ #define bar(y, z) (y + z)
+
**AlignConsecutiveAssignments** (``bool``)
If ``true``, aligns consecutive assignments.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits