[clang] 3e333cc - [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations
Björn Schäpers via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 3 08:55:01 PDT 2021
Author: Gerhard Gappmeier
Date: 2021-06-03T17:55:11+02:00
New Revision: 3e333cc82e42e1e2ecc974d896489eebe1a5edc2
URL: https://github.com/llvm/llvm-project/commit/3e333cc82e42e1e2ecc974d896489eebe1a5edc2
DIFF: https://github.com/llvm/llvm-project/commit/3e333cc82e42e1e2ecc974d896489eebe1a5edc2.diff
LOG: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations
This re-applies the old patch D27651, which was never landed, into the
latest "main" branch, without understanding the code. I just applied
the changes "mechanically" and made it compiling again.
This makes the right pointer alignment working as expected.
Fixes https://llvm.org/PR27353
For instance
const char* const* v1;
float const* v2;
SomeVeryLongType const& v3;
was formatted as
const char *const * v1;
float const * v2;
SomeVeryLongType const &v3;
This patch keep the *s or &s aligned to the right, next to their variable.
The above example is now formatted as
const char *const *v1;
float const *v2;
SomeVeryLongType const &v3;
It is a pity that this still does not work with clang-format in 2021,
even though there was a fix available in 2016. IMHO right pointer alignment
is the default case in C, because syntactically the pointer belongs to the
variable.
See
int* a, b, c; // wrong, just the 1st variable is a pointer
vs.
int *a, *b, *c; // right
Prominent example is the Linux kernel coding style.
Some styles argue the left pointer alignment is better and declaration
lists as shown above should be avoided. That's ok, as different projects
can use different styles, but this important style should work too.
I hope that somebody that has a better understanding about the code,
can take over this patch and land it into main.
For now I must maintain this fork to make it working for our projects.
Cheers,
Gerhard.
Differential Revision: https://reviews.llvm.org/D103245
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 11bcac075c241..6d3582a6297d6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -259,6 +259,9 @@ clang-format
- ``git-clang-format`` no longer formats changes to symbolic links. (Fixes
https://llvm.org/PR46992.)
+- Makes ``PointerAligment: Right`` working with ``AlignConsecutiveDeclarations``.
+ (Fixes https://llvm.org/PR27353)
+
libclang
--------
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index e78c5c4df5866..515cb7941e785 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -262,7 +262,8 @@ void WhitespaceManager::calculateLineBreakInformation() {
// Align a single sequence of tokens, see AlignTokens below.
template <typename F>
static void
-AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
+AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
+ unsigned Column, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
bool FoundMatchOnLine = false;
int Shift = 0;
@@ -365,9 +366,22 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
Changes[i].Spaces += Shift;
assert(Shift >= 0);
+
Changes[i].StartOfTokenColumn += Shift;
if (i + 1 != Changes.size())
Changes[i + 1].PreviousEndOfTokenColumn += Shift;
+
+ // If PointerAlignment is PAS_Right, keep *s or &s next to the token
+ if (Style.PointerAlignment == FormatStyle::PAS_Right &&
+ Changes[i].Spaces != 0) {
+ for (int Previous = i - 1;
+ Previous >= 0 &&
+ Changes[Previous].Tok->getType() == TT_PointerOrReference;
+ --Previous) {
+ Changes[Previous + 1].Spaces -= Shift;
+ Changes[Previous].Spaces += Shift;
+ }
+ }
}
}
@@ -437,8 +451,8 @@ static unsigned AlignTokens(
// containing any matching token to be aligned and located after such token.
auto AlignCurrentSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
- AlignTokenSequence(StartOfSequence, EndOfSequence, MinColumn, Matches,
- Changes);
+ AlignTokenSequence(Style, StartOfSequence, EndOfSequence, MinColumn,
+ Matches, Changes);
MinColumn = 0;
MaxColumn = UINT_MAX;
StartOfSequence = 0;
@@ -728,12 +742,6 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
if (Style.AlignConsecutiveDeclarations == FormatStyle::ACS_None)
return;
- // FIXME: Currently we don't handle properly the PointerAlignment: Right
- // The * and & are not aligned and are left dangling. Something has to be done
- // about it, but it raises the question of alignment of code like:
- // const char* const* v1;
- // float const* v2;
- // SomeVeryLongType const& v3;
AlignTokens(
Style,
[](Change const &C) {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index efabaabf6e7f5..79d838cb28b7a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -14915,6 +14915,7 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
FormatStyle Alignment = getLLVMStyle();
Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+ Alignment.PointerAlignment = FormatStyle::PAS_Right;
verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
Alignment);
@@ -14950,8 +14951,8 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
verifyFormat("int oneTwoThree{0}; // comment\n"
"unsigned oneTwo; // comment",
Alignment);
- verifyFormat("unsigned int * a;\n"
- "int * b;\n"
+ verifyFormat("unsigned int *a;\n"
+ "int *b;\n"
"unsigned int Const *c;\n"
"unsigned int const *d;\n"
"unsigned int Const &e;\n"
@@ -15073,9 +15074,11 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" double bar();\n"
"};\n",
Alignment);
+
+ // PAS_Right
EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
" int const i = 1;\n"
- " int * j = 2;\n"
+ " int *j = 2;\n"
" int big = 10000;\n"
"\n"
" unsigned oneTwoThree = 123;\n"
@@ -15096,6 +15099,161 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
"int ll=10000;\n"
"}",
Alignment));
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
+ " int **j = 2, ***k;\n"
+ " int &k = i;\n"
+ " int &&l = i + j;\n"
+ " int big = 10000;\n"
+ "\n"
+ " unsigned oneTwoThree = 123;\n"
+ " int oneTwo = 12;\n"
+ " method();\n"
+ " float k = 2;\n"
+ " int ll = 10000;\n"
+ "}",
+ format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int **j=2,***k;\n"
+ "int &k=i;\n"
+ "int &&l=i+j;\n"
+ " int big = 10000;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=10000;\n"
+ "}",
+ Alignment));
+ // variables are aligned at their name, pointers are at the right most
+ // position
+ verifyFormat("int *a;\n"
+ "int **b;\n"
+ "int ***c;\n"
+ "int foobar;\n",
+ Alignment);
+
+ // PAS_Left
+ FormatStyle AlignmentLeft = Alignment;
+ AlignmentLeft.PointerAlignment = FormatStyle::PAS_Left;
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
+ " int* j = 2;\n"
+ " int big = 10000;\n"
+ "\n"
+ " unsigned oneTwoThree = 123;\n"
+ " int oneTwo = 12;\n"
+ " method();\n"
+ " float k = 2;\n"
+ " int ll = 10000;\n"
+ "}",
+ format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int *j=2;\n"
+ " int big = 10000;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=10000;\n"
+ "}",
+ AlignmentLeft));
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
+ " int** j = 2;\n"
+ " int& k = i;\n"
+ " int&& l = i + j;\n"
+ " int big = 10000;\n"
+ "\n"
+ " unsigned oneTwoThree = 123;\n"
+ " int oneTwo = 12;\n"
+ " method();\n"
+ " float k = 2;\n"
+ " int ll = 10000;\n"
+ "}",
+ format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int **j=2;\n"
+ "int &k=i;\n"
+ "int &&l=i+j;\n"
+ " int big = 10000;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=10000;\n"
+ "}",
+ AlignmentLeft));
+ // variables are aligned at their name, pointers are at the left most position
+ verifyFormat("int* a;\n"
+ "int** b;\n"
+ "int*** c;\n"
+ "int foobar;\n",
+ AlignmentLeft);
+
+ // PAS_Middle
+ FormatStyle AlignmentMiddle = Alignment;
+ AlignmentMiddle.PointerAlignment = FormatStyle::PAS_Middle;
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
+ " int * j = 2;\n"
+ " int big = 10000;\n"
+ "\n"
+ " unsigned oneTwoThree = 123;\n"
+ " int oneTwo = 12;\n"
+ " method();\n"
+ " float k = 2;\n"
+ " int ll = 10000;\n"
+ "}",
+ format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int *j=2;\n"
+ " int big = 10000;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=10000;\n"
+ "}",
+ AlignmentMiddle));
+ EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
+ " int const i = 1;\n"
+ " int ** j = 2, ***k;\n"
+ " int & k = i;\n"
+ " int && l = i + j;\n"
+ " int big = 10000;\n"
+ "\n"
+ " unsigned oneTwoThree = 123;\n"
+ " int oneTwo = 12;\n"
+ " method();\n"
+ " float k = 2;\n"
+ " int ll = 10000;\n"
+ "}",
+ format("void SomeFunction(int parameter= 0) {\n"
+ " int const i= 1;\n"
+ " int **j=2,***k;\n"
+ "int &k=i;\n"
+ "int &&l=i+j;\n"
+ " int big = 10000;\n"
+ "\n"
+ "unsigned oneTwoThree =123;\n"
+ "int oneTwo = 12;\n"
+ " method();\n"
+ "float k= 2;\n"
+ "int ll=10000;\n"
+ "}",
+ AlignmentMiddle));
+ // variables are aligned at their name, pointers are in the middle
+ verifyFormat("int * a;\n"
+ "int * b;\n"
+ "int *** c;\n"
+ "int foobar;\n",
+ AlignmentMiddle);
+
Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_None;
Alignment.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
verifyFormat("#define A \\\n"
@@ -15129,7 +15287,7 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
Alignment);
verifyFormat("void SomeFunction(int parameter = 0) {\n"
" int const i = 1;\n"
- " int * j = 2;\n"
+ " int *j = 2;\n"
" int big = 10000;\n"
"}",
Alignment);
@@ -15232,7 +15390,7 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
" float b,\n"
" int c,\n"
" uint32_t *d) {\n"
- " int * e = 0;\n"
+ " int *e = 0;\n"
" float f = 0;\n"
" double g = 0;\n"
"}\n"
More information about the cfe-commits
mailing list