[clang] [clang-format] Fix a bug in `DerivePointerAlignment: true` (PR #150744)
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 27 21:36:32 PDT 2025
https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/150744
>From 7418fe966ff3a3e9f3c6c431beafbdde47a1de30 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sat, 26 Jul 2025 01:56:52 -0700
Subject: [PATCH 1/3] [clang-format] Fix a bug in `DerivePointerAlignment:
true`
This effectively reverts a4d4859dc70c046ad928805ddeaf8fa101793394
which didn't fix the problem that `int*,` was not counted as "Left"
alignment.
Fixes #150327
---
clang/lib/Format/Format.cpp | 31 ++++++++++-----------------
clang/unittests/Format/FormatTest.cpp | 7 +++++-
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 1cfa3d1535902..01d2b4dfc76cc 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2641,28 +2641,19 @@ class Formatter : public TokenAnalyzer {
int AlignmentDiff = 0;
for (const AnnotatedLine *Line : Lines) {
AlignmentDiff += countVariableAlignments(Line->Children);
- for (FormatToken *Tok = Line->First; Tok && Tok->Next; Tok = Tok->Next) {
+ const auto *Prev = Line->getFirstNonComment();
+ if (!Prev)
+ break;
+ for (const auto *Tok = Prev->Next; Tok; Prev = Tok, Tok = Tok->Next) {
if (Tok->isNot(TT_PointerOrReference))
continue;
- // Don't treat space in `void foo() &&` or `void() &&` as evidence.
- if (const auto *Prev = Tok->getPreviousNonComment()) {
- if (Prev->is(tok::r_paren) && Prev->MatchingParen) {
- if (const auto *Func =
- Prev->MatchingParen->getPreviousNonComment()) {
- if (Func->isOneOf(TT_FunctionDeclarationName, TT_StartOfName,
- TT_OverloadedOperator) ||
- Func->isTypeName(LangOpts)) {
- continue;
- }
- }
- }
- }
- bool SpaceBefore = Tok->hasWhitespaceBefore();
- bool SpaceAfter = Tok->Next->hasWhitespaceBefore();
- if (SpaceBefore && !SpaceAfter)
- ++AlignmentDiff;
- if (!SpaceBefore && SpaceAfter)
- --AlignmentDiff;
+ const auto *Next = Tok->Next;
+ if (!Next)
+ break;
+ if (Prev->Tok.getIdentifierInfo())
+ AlignmentDiff += Tok->hasWhitespaceBefore() ? 1 : -1;
+ if (Next->Tok.getIdentifierInfo())
+ AlignmentDiff += Next->hasWhitespaceBefore() ? -1 : 1;
}
}
return AlignmentDiff;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c20d099cac017..55360965d7575 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12114,7 +12114,12 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
"void b() const &;\n";
verifyFormat(Prefix + "int *x;", Prefix + "int* x;", DerivePointerAlignment);
- verifyGoogleFormat("MACRO(int*, std::function<void() &&>);");
+ constexpr StringRef Code("MACRO(int*, std::function<void() &&>);");
+ verifyFormat(Code, DerivePointerAlignment);
+
+ auto Style = getGoogleStyle();
+ Style.DerivePointerAlignment = true;
+ verifyFormat(Code, Style);
}
TEST_F(FormatTest, PointerAlignmentFallback) {
>From d56e24b91760c1d734aff1617057618343275abc Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sat, 26 Jul 2025 01:56:52 -0700
Subject: [PATCH 2/3] [clang-format] Fix a bug in `DerivePointerAlignment:
true`
This effectively reverts a4d4859dc70c046ad928805ddeaf8fa101793394
which didn't fix the problem that `int*,` was not counted as "Left"
alignment.
Fixes #150327
---
clang/lib/Format/Format.cpp | 39 +++++++++++++++++++++++++++----------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 38f78e80982e7..51c855c39d233 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2639,23 +2639,42 @@ class Formatter : public TokenAnalyzer {
int countVariableAlignments(const SmallVectorImpl<AnnotatedLine *> &Lines) {
int AlignmentDiff = 0;
+
for (const AnnotatedLine *Line : Lines) {
AlignmentDiff += countVariableAlignments(Line->Children);
- const auto *Prev = Line->getFirstNonComment();
- if (!Prev)
- break;
- for (const auto *Tok = Prev->Next; Tok; Prev = Tok, Tok = Tok->Next) {
+
+ for (const auto *Tok = Line->getFirstNonComment(); Tok; Tok = Tok->Next) {
if (Tok->isNot(TT_PointerOrReference))
continue;
+
+ const auto *Prev = Tok->Previous;
+ const bool PrecededByName = Prev && Prev->Tok.getIdentifierInfo();
+ const bool SpaceBefore = Tok->hasWhitespaceBefore();
+
+ // e.g. `int **`, `int*&`, etc.
+ while (Tok->Next && Tok->Next->is(TT_PointerOrReference))
+ Tok = Tok->Next;
+
const auto *Next = Tok->Next;
- if (!Next)
- break;
- if (Prev->Tok.getIdentifierInfo())
- AlignmentDiff += Tok->hasWhitespaceBefore() ? 1 : -1;
- if (Next->Tok.getIdentifierInfo())
- AlignmentDiff += Next->hasWhitespaceBefore() ? -1 : 1;
+ const bool FollowedByName = Next && Next->Tok.getIdentifierInfo();
+ const bool SpaceAfter = Next && Next->hasWhitespaceBefore();
+
+ if ((!PrecededByName && !FollowedByName) ||
+ // e.g. `int * i` or `int*i`
+ (PrecededByName && FollowedByName && SpaceBefore == SpaceAfter)) {
+ continue;
+ }
+
+ if ((PrecededByName && SpaceBefore) ||
+ (FollowedByName && !SpaceAfter)) {
+ ++AlignmentDiff;
+ } else if ((PrecededByName && !SpaceBefore) ||
+ (FollowedByName && SpaceAfter)) {
+ --AlignmentDiff;
+ }
}
}
+
return AlignmentDiff;
}
>From c1eb42af2684bc5e5a8175726cd18892e5ee09f3 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sun, 27 Jul 2025 21:34:49 -0700
Subject: [PATCH 3/3] Add comments
---
clang/lib/Format/Format.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 51c855c39d233..063780721423f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2667,9 +2667,11 @@ class Formatter : public TokenAnalyzer {
if ((PrecededByName && SpaceBefore) ||
(FollowedByName && !SpaceAfter)) {
+ // Right alignment.
++AlignmentDiff;
} else if ((PrecededByName && !SpaceBefore) ||
(FollowedByName && SpaceAfter)) {
+ // Left alignment.
--AlignmentDiff;
}
}
More information about the cfe-commits
mailing list