Author: Gedare Bloom Date: 2024-01-10T19:35:03-08:00 New Revision: b2c0c6f3f2741415d5257e16ca8d4083abe1b487
URL: https://github.com/llvm/llvm-project/commit/b2c0c6f3f2741415d5257e16ca8d4083abe1b487 DIFF: https://github.com/llvm/llvm-project/commit/b2c0c6f3f2741415d5257e16ca8d4083abe1b487.diff LOG: [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. Added: Modified: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/ConfigParseTest.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 3d42571e82d8a0..ac9a0b70ed5daa 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -392,6 +392,23 @@ the configuration (without a prefix: ``Auto``). a &= 2; bbb = 2; + * ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are + aligned. + + .. code-block:: c++ + + true: + unsigned i; + int &r; + int *p; + int (*f)(); + + false: + unsigned i; + int &r; + int *p; + int (*f)(); + * ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment operators are left-padded to the same length as long ones in order to put all assignment operators to the right of the left hand side. @@ -517,6 +534,23 @@ the configuration (without a prefix: ``Auto``). a &= 2; bbb = 2; + * ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are + aligned. + + .. code-block:: c++ + + true: + unsigned i; + int &r; + int *p; + int (*f)(); + + false: + unsigned i; + int &r; + int *p; + int (*f)(); + * ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment operators are left-padded to the same length as long ones in order to put all assignment operators to the right of the left hand side. @@ -642,6 +676,23 @@ the configuration (without a prefix: ``Auto``). a &= 2; bbb = 2; + * ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are + aligned. + + .. code-block:: c++ + + true: + unsigned i; + int &r; + int *p; + int (*f)(); + + false: + unsigned i; + int &r; + int *p; + int (*f)(); + * ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment operators are left-padded to the same length as long ones in order to put all assignment operators to the right of the left hand side. @@ -768,6 +819,23 @@ the configuration (without a prefix: ``Auto``). a &= 2; bbb = 2; + * ``bool AlignFunctionPointers`` Only for ``AlignConsecutiveDeclarations``. Whether function pointers are + aligned. + + .. code-block:: c++ + + true: + unsigned i; + int &r; + int *p; + int (*f)(); + + false: + unsigned i; + int &r; + int *p; + int (*f)(); + * ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment operators are left-padded to the same length as long ones in order to put all assignment operators to the right of the left hand side. diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ade0036ba2fd6d..7abc65b8734a0f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1094,6 +1094,7 @@ clang-format - Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property attributes (like ``nonatomic, strong, nullable``). - Add ``.clang-format-ignore`` files. +- Add ``AlignFunctionPointers`` sub-option for ``AlignConsecutiveDeclarations``. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 8604dea689f937..59b645ecab715b 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -225,6 +225,22 @@ struct FormatStyle { /// bbb = 2; /// \endcode bool AlignCompound; + /// Only for ``AlignConsecutiveDeclarations``. Whether function pointers are + /// aligned. + /// \code + /// true: + /// unsigned i; + /// int &r; + /// int *p; + /// int (*f)(); + /// + /// false: + /// unsigned i; + /// int &r; + /// int *p; + /// int (*f)(); + /// \endcode + bool AlignFunctionPointers; /// Only for ``AlignConsecutiveAssignments``. Whether short assignment /// operators are left-padded to the same length as long ones in order to /// put all assignment operators to the right of the left hand side. @@ -247,7 +263,9 @@ struct FormatStyle { bool operator==(const AlignConsecutiveStyle &R) const { return Enabled == R.Enabled && AcrossEmptyLines == R.AcrossEmptyLines && AcrossComments == R.AcrossComments && - AlignCompound == R.AlignCompound && PadOperators == R.PadOperators; + AlignCompound == R.AlignCompound && + AlignFunctionPointers == R.AlignFunctionPointers && + PadOperators == R.PadOperators; } bool operator!=(const AlignConsecutiveStyle &R) const { return !(*this == R); diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index f798d555bf9929..ff5ed6c306f383 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -76,41 +76,39 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> { FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/false, /*AcrossEmptyLines=*/false, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); IO.enumCase(Value, "Consecutive", FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/true, /*AcrossEmptyLines=*/false, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); IO.enumCase(Value, "AcrossEmptyLines", FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/true, /*AcrossEmptyLines=*/true, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); IO.enumCase(Value, "AcrossComments", - FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true, - /*AcrossEmptyLines=*/false, - /*AcrossComments=*/true, - /*AlignCompound=*/false, - /*PadOperators=*/true})); + FormatStyle::AlignConsecutiveStyle( + {/*Enabled=*/true, /*AcrossEmptyLines=*/false, + /*AcrossComments=*/true, /*AlignCompound=*/false, + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); IO.enumCase(Value, "AcrossEmptyLinesAndComments", - FormatStyle::AlignConsecutiveStyle({/*Enabled=*/true, - /*AcrossEmptyLines=*/true, - /*AcrossComments=*/true, - /*AlignCompound=*/false, - /*PadOperators=*/true})); + FormatStyle::AlignConsecutiveStyle( + {/*Enabled=*/true, /*AcrossEmptyLines=*/true, + /*AcrossComments=*/true, /*AlignCompound=*/false, + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); // For backward compatibility. IO.enumCase(Value, "true", FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/true, /*AcrossEmptyLines=*/false, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); IO.enumCase(Value, "false", FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/false, /*AcrossEmptyLines=*/false, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); } static void mapping(IO &IO, FormatStyle::AlignConsecutiveStyle &Value) { @@ -118,6 +116,7 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> { IO.mapOptional("AcrossEmptyLines", Value.AcrossEmptyLines); IO.mapOptional("AcrossComments", Value.AcrossComments); IO.mapOptional("AlignCompound", Value.AlignCompound); + IO.mapOptional("AlignFunctionPointers", Value.AlignFunctionPointers); IO.mapOptional("PadOperators", Value.PadOperators); } }; @@ -1432,6 +1431,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.AlignConsecutiveAssignments.AcrossEmptyLines = false; LLVMStyle.AlignConsecutiveAssignments.AcrossComments = false; LLVMStyle.AlignConsecutiveAssignments.AlignCompound = false; + LLVMStyle.AlignConsecutiveAssignments.AlignFunctionPointers = false; LLVMStyle.AlignConsecutiveAssignments.PadOperators = true; LLVMStyle.AlignConsecutiveBitFields = {}; LLVMStyle.AlignConsecutiveDeclarations = {}; diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 3bc6915b8df0a7..f1d176f182ffa4 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -978,7 +978,14 @@ void WhitespaceManager::alignConsecutiveDeclarations() { AlignTokens( Style, - [](Change const &C) { + [&](Change const &C) { + if (Style.AlignConsecutiveDeclarations.AlignFunctionPointers) { + for (const auto *Prev = C.Tok->Previous; Prev; Prev = Prev->Previous) + if (Prev->is(tok::equal)) + return false; + if (C.Tok->is(TT_FunctionTypeLParen)) + return true; + } if (C.Tok->is(TT_FunctionDeclarationName)) return true; if (C.Tok->isNot(TT_StartOfName)) diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 0c9f68f303d865..18ecba270e3455 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -289,37 +289,43 @@ TEST(ConfigParseTest, ParsesConfiguration) { #define CHECK_ALIGN_CONSECUTIVE(FIELD) \ do { \ Style.FIELD.Enabled = true; \ - CHECK_PARSE(#FIELD ": None", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \ - /*AcrossComments=*/false, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ - CHECK_PARSE(#FIELD ": Consecutive", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \ - /*AcrossComments=*/false, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ - CHECK_PARSE(#FIELD ": AcrossEmptyLines", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \ - /*AcrossComments=*/false, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ - CHECK_PARSE(#FIELD ": AcrossEmptyLinesAndComments", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \ - /*AcrossComments=*/true, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": None", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \ + /*AcrossComments=*/false, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": Consecutive", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \ + /*AcrossComments=*/false, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": AcrossEmptyLines", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \ + /*AcrossComments=*/false, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": AcrossEmptyLinesAndComments", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \ + /*AcrossComments=*/true, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ /* For backwards compability, false / true should still parse */ \ - CHECK_PARSE(#FIELD ": false", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \ - /*AcrossComments=*/false, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ - CHECK_PARSE(#FIELD ": true", FIELD, \ - FormatStyle::AlignConsecutiveStyle( \ - {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \ - /*AcrossComments=*/false, /*AlignCompound=*/false, \ - /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": false", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \ + /*AcrossComments=*/false, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ + CHECK_PARSE( \ + #FIELD ": true", FIELD, \ + FormatStyle::AlignConsecutiveStyle( \ + {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \ + /*AcrossComments=*/false, /*AlignCompound=*/false, \ + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \ \ CHECK_PARSE_NESTED_BOOL(FIELD, Enabled); \ CHECK_PARSE_NESTED_BOOL(FIELD, AcrossEmptyLines); \ diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index c346f382b34314..9fd55db44df685 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -2046,13 +2046,26 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) { Style); Style.AlignConsecutiveDeclarations.Enabled = true; + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = true; verifyFormat("Const unsigned int *c;\n" "const unsigned int *d;\n" "Const unsigned int &e;\n" "const unsigned int &f;\n" + "int *f1(int *a, int &b, int &&c);\n" + "double *(*f2)(int *a, double &&b);\n" "const unsigned &&g;\n" "Const unsigned h;", Style); + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = false; + verifyFormat("Const unsigned int *c;\n" + "const unsigned int *d;\n" + "Const unsigned int &e;\n" + "const unsigned int &f;\n" + "int *f1(int *a, int &b, int &&c);\n" + "double *(*f2)(int *a, double &&b);\n" + "const unsigned &&g;\n" + "Const unsigned h;", + Style); Style.PointerAlignment = FormatStyle::PAS_Left; Style.ReferenceAlignment = FormatStyle::RAS_Pointer; @@ -2091,13 +2104,26 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) { Style); Style.AlignConsecutiveDeclarations.Enabled = true; + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = true; verifyFormat("Const unsigned int* c;\n" "const unsigned int* d;\n" "Const unsigned int& e;\n" "const unsigned int& f;\n" + "int* f1(int* a, int& b, int&& c);\n" + "double* (*f2)(int* a, double&& b);\n" "const unsigned&& g;\n" "Const unsigned h;", Style); + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = false; + verifyFormat("Const unsigned int* c;\n" + "const unsigned int* d;\n" + "Const unsigned int& e;\n" + "const unsigned int& f;\n" + "int* f1(int* a, int& b, int&& c);\n" + "double* (*f2)(int* a, double&& b);\n" + "const unsigned&& g;\n" + "Const unsigned h;", + Style); Style.PointerAlignment = FormatStyle::PAS_Right; Style.ReferenceAlignment = FormatStyle::RAS_Left; @@ -2116,13 +2142,26 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) { verifyFormat("for (int a = 0, b++; const Foo *c : {1, 2, 3})", Style); Style.AlignConsecutiveDeclarations.Enabled = true; + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = true; verifyFormat("Const unsigned int *c;\n" "const unsigned int *d;\n" "Const unsigned int& e;\n" "const unsigned int& f;\n" - "const unsigned g;\n" + "int *f1(int *a, int& b, int&& c);\n" + "double *(*f2)(int *a, double&& b);\n" + "const unsigned&& g;\n" "Const unsigned h;", Style); + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = false; + verifyFormat("Const unsigned int *c;\n" + "const unsigned int *d;\n" + "Const unsigned int& e;\n" + "const unsigned int& f;\n" + "int *f1(int *a, int& b, int&& c);\n" + "double *(*f2)(int *a, double&& b);\n" + "const unsigned&& g;\n" + "Const unsigned h;", + Style); Style.PointerAlignment = FormatStyle::PAS_Left; Style.ReferenceAlignment = FormatStyle::RAS_Middle; @@ -2156,13 +2195,26 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) { Style); Style.AlignConsecutiveDeclarations.Enabled = true; + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = true; verifyFormat("Const unsigned int* c;\n" "const unsigned int* d;\n" "Const unsigned int & e;\n" "const unsigned int & f;\n" + "int* f1(int* a, int & b, int && c);\n" + "double* (*f2)(int* a, double && b);\n" "const unsigned && g;\n" "Const unsigned h;", Style); + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = false; + verifyFormat("Const unsigned int* c;\n" + "const unsigned int* d;\n" + "Const unsigned int & e;\n" + "const unsigned int & f;\n" + "int* f1(int* a, int & b, int && c);\n" + "double* (*f2)(int* a, double && b);\n" + "const unsigned && g;\n" + "Const unsigned h;", + Style); Style.PointerAlignment = FormatStyle::PAS_Middle; Style.ReferenceAlignment = FormatStyle::RAS_Right; @@ -2181,13 +2233,26 @@ TEST_F(FormatTest, SeparatePointerReferenceAlignment) { verifyFormat("for (int a = 0, b++; const Foo * c : {1, 2, 3})", Style); Style.AlignConsecutiveDeclarations.Enabled = true; + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = true; verifyFormat("Const unsigned int * c;\n" "const unsigned int * d;\n" "Const unsigned int &e;\n" "const unsigned int &f;\n" + "int * f1(int * a, int &b, int &&c);\n" + "double * (*f2)(int * a, double &&b);\n" "const unsigned &&g;\n" "Const unsigned h;", Style); + Style.AlignConsecutiveDeclarations.AlignFunctionPointers = false; + verifyFormat("Const unsigned int * c;\n" + "const unsigned int * d;\n" + "Const unsigned int &e;\n" + "const unsigned int &f;\n" + "int * f1(int * a, int &b, int &&c);\n" + "double * (*f2)(int * a, double &&b);\n" + "const unsigned &&g;\n" + "Const unsigned h;", + Style); // FIXME: we don't handle this yet, so output may be arbitrary until it's // specifically handled @@ -18933,6 +18998,15 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { " \"bb\"};\n" "int bbbbbbb = 0;", Alignment); + // http://llvm.org/PR68079 + verifyFormat("using Fn = int (A::*)();\n" + "using RFn = int (A::*)() &;\n" + "using RRFn = int (A::*)() &&;", + Alignment); + verifyFormat("using Fn = int (A::*)();\n" + "using RFn = int *(A::*)() &;\n" + "using RRFn = double (A::*)() &&;", + Alignment); // PAS_Right verifyFormat("void SomeFunction(int parameter = 0) {\n" @@ -19585,7 +19659,7 @@ TEST_F(FormatTest, AlignWithLineBreaks) { FormatStyle::AlignConsecutiveStyle( {/*Enabled=*/false, /*AcrossEmptyLines=*/false, /*AcrossComments=*/false, /*AlignCompound=*/false, - /*PadOperators=*/true})); + /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); EXPECT_EQ(Style.AlignConsecutiveDeclarations, FormatStyle::AlignConsecutiveStyle({})); verifyFormat("void foo() {\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
