Skip to content

Commit b2c0c6f

Browse files
gedaregedare
authored
[clang-format]: Split alignment of declarations around assignment (#69340)
Function pointers are detected as a type of declaration using FunctionTypeLParen. They are aligned based on rules for AlignConsecutiveDeclarations. When a function pointer is on the right-hand side of an assignment, the alignment of the function pointer can result in excessive whitespace padding due to the ordering of alignment, as the alignment processes a line from left-to-right and first aligns the declarations before and after the assignment operator, and then aligns the assignment operator. Injection of whitespace by alignment of declarations after the equal sign followed by alignment of the equal sign results in the excessive whitespace. Fixes #68079.
1 parent cc77e33 commit b2c0c6f

File tree

7 files changed

+223
-49
lines changed

7 files changed

+223
-49
lines changed

‎clang/docs/ClangFormatStyleOptions.rst

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,23 @@ the configuration (without a prefix: ``Auto``).
392392
a &= 2;
393393
bbb = 2;
394394

395+
* ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
396+
aligned.
397+
398+
.. code-block:: c++
399+
400+
true:
401+
unsigned i;
402+
int &r;
403+
int *p;
404+
int (*f)();
405+
406+
false:
407+
unsigned i;
408+
int &r;
409+
int *p;
410+
int (*f)();
411+
395412
* ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment
396413
operators are left-padded to the same length as long ones in order to
397414
put all assignment operators to the right of the left hand side.
@@ -517,6 +534,23 @@ the configuration (without a prefix: ``Auto``).
517534
a &= 2;
518535
bbb = 2;
519536

537+
* ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
538+
aligned.
539+
540+
.. code-block:: c++
541+
542+
true:
543+
unsigned i;
544+
int &r;
545+
int *p;
546+
int (*f)();
547+
548+
false:
549+
unsigned i;
550+
int &r;
551+
int *p;
552+
int (*f)();
553+
520554
* ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment
521555
operators are left-padded to the same length as long ones in order to
522556
put all assignment operators to the right of the left hand side.
@@ -642,6 +676,23 @@ the configuration (without a prefix: ``Auto``).
642676
a &= 2;
643677
bbb = 2;
644678

679+
* ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
680+
aligned.
681+
682+
.. code-block:: c++
683+
684+
true:
685+
unsigned i;
686+
int &r;
687+
int *p;
688+
int (*f)();
689+
690+
false:
691+
unsigned i;
692+
int &r;
693+
int *p;
694+
int (*f)();
695+
645696
* ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment
646697
operators are left-padded to the same length as long ones in order to
647698
put all assignment operators to the right of the left hand side.
@@ -768,6 +819,23 @@ the configuration (without a prefix: ``Auto``).
768819
a &= 2;
769820
bbb = 2;
770821

822+
* ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
823+
aligned.
824+
825+
.. code-block:: c++
826+
827+
true:
828+
unsigned i;
829+
int &r;
830+
int *p;
831+
int (*f)();
832+
833+
false:
834+
unsigned i;
835+
int &r;
836+
int *p;
837+
int (*f)();
838+
771839
* ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment
772840
operators are left-padded to the same length as long ones in order to
773841
put all assignment operators to the right of the left hand side.

‎clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,7 @@ clang-format
10941094
- Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property
10951095
attributes (like ``nonatomic, strong, nullable``).
10961096
- Add ``.clang-format-ignore`` files.
1097+
- Add ``AlignFunctionPointers`` sub-option for ``AlignConsecutiveDeclarations``.
10971098

10981099
libclang
10991100
--------

‎clang/include/clang/Format/Format.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,22 @@ struct FormatStyle {
225225
/// bbb = 2;
226226
/// \endcode
227227
bool AlignCompound;
228+
/// Only for ``AlignConsecutiveDeclarations``. Whether function pointers are
229+
/// aligned.
230+
/// \code
231+
/// true:
232+
/// unsigned i;
233+
/// int &r;
234+
/// int *p;
235+
/// int (*f)();
236+
///
237+
/// false:
238+
/// unsigned i;
239+
/// int &r;
240+
/// int *p;
241+
/// int (*f)();
242+
/// \endcode
243+
bool AlignFunctionPointers;
228244
/// Only for ``AlignConsecutiveAssignments``. Whether short assignment
229245
/// operators are left-padded to the same length as long ones in order to
230246
/// put all assignment operators to the right of the left hand side.
@@ -247,7 +263,9 @@ struct FormatStyle {
247263
bool operator==(const AlignConsecutiveStyle &R) const {
248264
return Enabled == R.Enabled && AcrossEmptyLines == R.AcrossEmptyLines &&
249265
AcrossComments == R.AcrossComments &&
250-
AlignCompound == R.AlignCompound && PadOperators == R.PadOperators;
266+
AlignCompound == R.AlignCompound &&
267+
AlignFunctionPointers == R.AlignFunctionPointers &&
268+
PadOperators == R.PadOperators;
251269
}
252270
bool operator!=(const AlignConsecutiveStyle &R) const {
253271
return !(*this == R);

‎clang/lib/Format/Format.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,48 +76,47 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> {
7676
FormatStyle::AlignConsecutiveStyle(
7777
{/*Enabled=*/false, /*AcrossEmptyLines=*/false,
7878
/*AcrossComments=*/false, /*AlignCompound=*/false,
79-
/*PadOperators=*/true}));
79+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
8080
IO.enumCase(Value, "Consecutive",
8181
FormatStyle::AlignConsecutiveStyle(
8282
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
8383
/*AcrossComments=*/false, /*AlignCompound=*/false,
84-
/*PadOperators=*/true}));
84+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
8585
IO.enumCase(Value, "AcrossEmptyLines",
8686
FormatStyle::AlignConsecutiveStyle(
8787
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
8888
/*AcrossComments=*/false, /*AlignCompound=*/false,
89-
/*PadOperators=*/true}));
89+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
9090
IO.enumCase(Value, "AcrossComments",
91-
FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
92-
/*AcrossEmptyLines=*/false,
93-
/*AcrossComments=*/true,
94-
/*AlignCompound=*/false,
95-
/*PadOperators=*/true}));
91+
FormatStyle::AlignConsecutiveStyle(
92+
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
93+
/*AcrossComments=*/true, /*AlignCompound=*/false,
94+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
9695
IO.enumCase(Value, "AcrossEmptyLinesAndComments",
97-
FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true,
98-
/*AcrossEmptyLines=*/true,
99-
/*AcrossComments=*/true,
100-
/*AlignCompound=*/false,
101-
/*PadOperators=*/true}));
96+
FormatStyle::AlignConsecutiveStyle(
97+
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
98+
/*AcrossComments=*/true, /*AlignCompound=*/false,
99+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
102100

103101
// For backward compatibility.
104102
IO.enumCase(Value, "true",
105103
FormatStyle::AlignConsecutiveStyle(
106104
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
107105
/*AcrossComments=*/false, /*AlignCompound=*/false,
108-
/*PadOperators=*/true}));
106+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
109107
IO.enumCase(Value, "false",
110108
FormatStyle::AlignConsecutiveStyle(
111109
{/*Enabled=*/false, /*AcrossEmptyLines=*/false,
112110
/*AcrossComments=*/false, /*AlignCompound=*/false,
113-
/*PadOperators=*/true}));
111+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
114112
}
115113

116114
static void mapping(IO &IO, FormatStyle::AlignConsecutiveStyle &Value) {
117115
IO.mapOptional("Enabled", Value.Enabled);
118116
IO.mapOptional("AcrossEmptyLines", Value.AcrossEmptyLines);
119117
IO.mapOptional("AcrossComments", Value.AcrossComments);
120118
IO.mapOptional("AlignCompound", Value.AlignCompound);
119+
IO.mapOptional("AlignFunctionPointers", Value.AlignFunctionPointers);
121120
IO.mapOptional("PadOperators", Value.PadOperators);
122121
}
123122
};
@@ -1432,6 +1431,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
14321431
LLVMStyle.AlignConsecutiveAssignments.AcrossEmptyLines = false;
14331432
LLVMStyle.AlignConsecutiveAssignments.AcrossComments = false;
14341433
LLVMStyle.AlignConsecutiveAssignments.AlignCompound = false;
1434+
LLVMStyle.AlignConsecutiveAssignments.AlignFunctionPointers = false;
14351435
LLVMStyle.AlignConsecutiveAssignments.PadOperators = true;
14361436
LLVMStyle.AlignConsecutiveBitFields = {};
14371437
LLVMStyle.AlignConsecutiveDeclarations = {};

‎clang/lib/Format/WhitespaceManager.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,14 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
978978

979979
AlignTokens(
980980
Style,
981-
[](Change const &C) {
981+
[&](Change const &C) {
982+
if (Style.AlignConsecutiveDeclarations.AlignFunctionPointers) {
983+
for (const auto *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous)
984+
if (Prev->is(tok::equal))
985+
return false;
986+
if (C.Tok->is(TT_FunctionTypeLParen))
987+
return true;
988+
}
982989
if (C.Tok->is(TT_FunctionDeclarationName))
983990
return true;
984991
if (C.Tok->isNot(TT_StartOfName))

‎clang/unittests/Format/ConfigParseTest.cpp

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -289,37 +289,43 @@ TEST(ConfigParseTest, ParsesConfiguration) {
289289
#define CHECK_ALIGN_CONSECUTIVE(FIELD) \
290290
do { \
291291
Style.FIELD.Enabled = true; \
292-
CHECK_PARSE(#FIELD ": None", FIELD, \
293-
FormatStyle::AlignConsecutiveStyle( \
294-
{/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
295-
/*AcrossComments=*/false, /*AlignCompound=*/false, \
296-
/*PadOperators=*/true})); \
297-
CHECK_PARSE(#FIELD ": Consecutive", FIELD, \
298-
FormatStyle::AlignConsecutiveStyle( \
299-
{/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
300-
/*AcrossComments=*/false, /*AlignCompound=*/false, \
301-
/*PadOperators=*/true})); \
302-
CHECK_PARSE(#FIELD ": AcrossEmptyLines", FIELD, \
303-
FormatStyle::AlignConsecutiveStyle( \
304-
{/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
305-
/*AcrossComments=*/false, /*AlignCompound=*/false, \
306-
/*PadOperators=*/true})); \
307-
CHECK_PARSE(#FIELD ": AcrossEmptyLinesAndComments", FIELD, \
308-
FormatStyle::AlignConsecutiveStyle( \
309-
{/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
310-
/*AcrossComments=*/true, /*AlignCompound=*/false, \
311-
/*PadOperators=*/true})); \
292+
CHECK_PARSE( \
293+
#FIELD ": None", FIELD, \
294+
FormatStyle::AlignConsecutiveStyle( \
295+
{/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
296+
/*AcrossComments=*/false, /*AlignCompound=*/false, \
297+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
298+
CHECK_PARSE( \
299+
#FIELD ": Consecutive", FIELD, \
300+
FormatStyle::AlignConsecutiveStyle( \
301+
{/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
302+
/*AcrossComments=*/false, /*AlignCompound=*/false, \
303+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
304+
CHECK_PARSE( \
305+
#FIELD ": AcrossEmptyLines", FIELD, \
306+
FormatStyle::AlignConsecutiveStyle( \
307+
{/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
308+
/*AcrossComments=*/false, /*AlignCompound=*/false, \
309+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
310+
CHECK_PARSE( \
311+
#FIELD ": AcrossEmptyLinesAndComments", FIELD, \
312+
FormatStyle::AlignConsecutiveStyle( \
313+
{/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
314+
/*AcrossComments=*/true, /*AlignCompound=*/false, \
315+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
312316
/* For backwards compability, false / true should still parse */ \
313-
CHECK_PARSE(#FIELD ": false", FIELD, \
314-
FormatStyle::AlignConsecutiveStyle( \
315-
{/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
316-
/*AcrossComments=*/false, /*AlignCompound=*/false, \
317-
/*PadOperators=*/true})); \
318-
CHECK_PARSE(#FIELD ": true", FIELD, \
319-
FormatStyle::AlignConsecutiveStyle( \
320-
{/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
321-
/*AcrossComments=*/false, /*AlignCompound=*/false, \
322-
/*PadOperators=*/true})); \
317+
CHECK_PARSE( \
318+
#FIELD ": false", FIELD, \
319+
FormatStyle::AlignConsecutiveStyle( \
320+
{/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
321+
/*AcrossComments=*/false, /*AlignCompound=*/false, \
322+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
323+
CHECK_PARSE( \
324+
#FIELD ": true", FIELD, \
325+
FormatStyle::AlignConsecutiveStyle( \
326+
{/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
327+
/*AcrossComments=*/false, /*AlignCompound=*/false, \
328+
/*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
323329
\
324330
CHECK_PARSE_NESTED_BOOL(FIELD, Enabled); \
325331
CHECK_PARSE_NESTED_BOOL(FIELD, AcrossEmptyLines); \

0 commit comments

Comments
 (0)