[clang-format] Reuse AlignTokens for aligning macros #164120
Merged
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
|
@llvm/pr-subscribers-clang-format Author: Björn Schäpers (HazardyKnusperkeks) ChangesFixes #52985. This leaves aligning short case statements with its own logic. But that is harder to port, because it aligns even with no content of the right hand side of the :. Full diff: https://github.com/llvm/llvm-project/pull/164120.diff 2 Files Affected:
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 7348a3af8cf95..19984fdfa98fd 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -432,7 +432,11 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
// right-justified. It is used to align compound assignments like `+=` and `=`.
// When RightJustify and ACS.PadOperators are true, operators in each block to
// be aligned will be padded on the left to the same length before aligning.
-template <typename F>
+//
+// The simple check will not look at the indentaion and nesting level to recurse
+// into the line for alignment. It will also not count the commas. This is e.g.
+// for aligning macro definitions.
+template <typename F, bool SimpleCheck = false>
static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes,
unsigned StartAt,
@@ -465,9 +469,9 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
// Measure the scope level (i.e. depth of (), [], {}) of the first token, and
// abort when we hit any token in a higher scope than the starting one.
- auto IndentAndNestingLevel = StartAt < Changes.size()
- ? Changes[StartAt].indentAndNestingLevel()
- : std::tuple<unsigned, unsigned, unsigned>();
+ const auto IndentAndNestingLevel =
+ StartAt < Changes.size() ? Changes[StartAt].indentAndNestingLevel()
+ : std::tuple<unsigned, unsigned, unsigned>();
// 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
@@ -536,14 +540,17 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
if (CurrentChange.Tok->isNot(tok::comment))
LineIsComment = false;
- if (CurrentChange.Tok->is(tok::comma)) {
- ++CommasBeforeMatch;
- } else if (CurrentChange.indentAndNestingLevel() > IndentAndNestingLevel) {
- // Call AlignTokens recursively, skipping over this scope block.
- unsigned StoppedAt =
- AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
- i = StoppedAt - 1;
- continue;
+ if (!SimpleCheck) {
+ if (CurrentChange.Tok->is(tok::comma)) {
+ ++CommasBeforeMatch;
+ } else if (CurrentChange.indentAndNestingLevel() >
+ IndentAndNestingLevel) {
+ // Call AlignTokens recursively, skipping over this scope block.
+ unsigned StoppedAt =
+ AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
+ i = StoppedAt - 1;
+ continue;
+ }
}
if (!Matches(CurrentChange))
@@ -683,61 +690,8 @@ void WhitespaceManager::alignConsecutiveMacros() {
return Current->Next->SpacesRequiredBefore == SpacesRequiredBefore;
};
- unsigned MinColumn = 0;
-
- // Start and end of the token sequence we're processing.
- unsigned StartOfSequence = 0;
- unsigned EndOfSequence = 0;
-
- // Whether a matching token has been found on the current line.
- bool FoundMatchOnLine = false;
-
- // Whether the current line consists only of comments
- bool LineIsComment = true;
-
- unsigned I = 0;
- for (unsigned E = Changes.size(); I != E; ++I) {
- if (Changes[I].NewlinesBefore != 0) {
- EndOfSequence = I;
-
- // Whether to break the alignment sequence because of an empty line.
- bool EmptyLineBreak = (Changes[I].NewlinesBefore > 1) &&
- !Style.AlignConsecutiveMacros.AcrossEmptyLines;
-
- // Whether to break the alignment sequence because of a line without a
- // match.
- bool NoMatchBreak =
- !FoundMatchOnLine &&
- !(LineIsComment && Style.AlignConsecutiveMacros.AcrossComments);
-
- if (EmptyLineBreak || NoMatchBreak) {
- AlignMatchingTokenSequence(StartOfSequence, EndOfSequence, MinColumn,
- AlignMacrosMatches, Changes);
- }
-
- // A new line starts, re-initialize line status tracking bools.
- FoundMatchOnLine = false;
- LineIsComment = true;
- }
-
- if (Changes[I].Tok->isNot(tok::comment))
- LineIsComment = false;
-
- if (!AlignMacrosMatches(Changes[I]))
- continue;
-
- FoundMatchOnLine = true;
-
- if (StartOfSequence == 0)
- StartOfSequence = I;
-
- unsigned ChangeMinColumn = Changes[I].StartOfTokenColumn;
- MinColumn = std::max(MinColumn, ChangeMinColumn);
- }
-
- EndOfSequence = I;
- AlignMatchingTokenSequence(StartOfSequence, EndOfSequence, MinColumn,
- AlignMacrosMatches, Changes);
+ AlignTokens<decltype(AlignMacrosMatches)&, /*DontRecurse=*/true>(
+ Style, AlignMacrosMatches, Changes, 0, Style.AlignConsecutiveMacros);
}
void WhitespaceManager::alignConsecutiveAssignments() {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index b9ad930164944..3564a8ede0a01 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18559,6 +18559,11 @@ TEST_F(FormatTest, AlignConsecutiveMacros) {
"#define bbbb 4\n"
"#define ccc (5)",
Style);
+
+ Style.ColumnLimit = 30;
+ verifyFormat("#define MY_FUNC(x) callMe(X)\n"
+ "#define MY_LONG_CONSTANT 17",
+ Style);
}
TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Fixes llvm#52985. This leaves aligning short case statements with its own logic. But that is harder to port, because it aligns even with no content of the right hand side of the :.
HazardyKnusperkeks
force-pushed
the
macro
branch
from
799514e to
25469d9
Compare
owenca
approved these changes
Oct 19, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/15547 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/213/builds/1046 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes #52985.
This leaves aligning short case statements with its own logic. But that is harder to port, because it aligns even with no content of the right hand side of the :.