r372919 - [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.
Mikael Holmén via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 25 23:50:21 PDT 2019
Fixed the testcase in r372944.
On 2019-09-26 08:14, Mikael Holmén via cfe-commits wrote:
> Hi,
>
> On 2019-09-26 03:16, Evgenii Stepanov via cfe-commits wrote:
>> Hi,
>>
>> this change breaks the build with
>>
>> /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/clang/lib/Format/Format.cpp:737:44:
>> error: missing field 'SortPriority' initializer
>> [-Werror,-Wmissing-field-initializers]
>> {"^\"(llvm|llvm-c|clang|clang-c)/", 2},
>
> Same thing with a testcase:
>
> /data/repo/master/clang/unittests/Format/SortIncludesTest.cpp:522:60:
> error: missing field 'SortPriority' initializer
> [-Werror,-Wmissing-field-initializers]
> Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
> ^
> /data/repo/master/clang/unittests/Format/SortIncludesTest.cpp:522:71:
> error: missing field 'SortPriority' initializer
> [-Werror,-Wmissing-field-initializers]
> Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
> ^
> /data/repo/master/clang/unittests/Format/SortIncludesTest.cpp:542:60:
> error: missing field 'SortPriority' initializer
> [-Werror,-Wmissing-field-initializers]
> Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
> ^
> /data/repo/master/clang/unittests/Format/SortIncludesTest.cpp:542:71:
> error: missing field 'SortPriority' initializer
> [-Werror,-Wmissing-field-initializers]
> Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
> ^
> 4 errors generated.
>
> /Mikael
>
>
>> https://protect2.fireeye.com/url?k=7ed71e57-220315e9-7ed75ecc-86742d02e7e2-ebf5547e7dd70b16&q=1&u=http%3A%2F%2Flab.llvm.org%3A8011%2Fbuilders%2Fsanitizer-x86_64-linux-android%2Fbuilds%2F24667%2Fsteps%2Fbootstrap%2520clang%2Flogs%2Fstdio
>>
>> On Wed, Sep 25, 2019 at 1:30 PM Paul Hoad via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>> Author: paulhoad
>>> Date: Wed Sep 25 13:33:01 2019
>>> New Revision: 372919
>>>
>>> URL: https://protect2.fireeye.com/url?k=ce962f68-924224d6-ce966ff3-86742d02e7e2-1bc81aee6f2698ad&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D372919%26view%3Drev
>>> Log:
>>> [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.
>>>
>>> Summary:
>>> This new Style rule is made as a part of adding support for NetBSD KNF in clang-format. NetBSD have it's own priority of includes which should be followed while formatting NetBSD code. This style sorts the Cpp Includes according to the priorities of NetBSD, as mentioned in the [Style Guide](https://protect2.fireeye.com/url?k=c8a250e5-94765b5b-c8a2107e-86742d02e7e2-6cc3f1b124909847&q=1&u=http%3A%2F%2Fcvsweb.netbsd.org%2Fbsdweb.cgi%2Fsrc%2Fshare%2Fmisc%2Fstyle%3Frev%3DHEAD%26content-type%3Dtext%2Fx-cvsweb-markup)
>>> The working of this Style rule shown below:
>>>
>>> **Configuration:**
>>> This revision introduces a new field under IncludeCategories named `SortPriority` which defines the priority of ordering the `#includes` and the `Priority` will define the categories for grouping the `#include blocks`.
>>>
>>> Reviewers: cfe-commits, mgorny, christos, MyDeveloperDay
>>>
>>> Reviewed By: MyDeveloperDay
>>>
>>> Subscribers: lebedev.ri, rdwampler, christos, mgorny, krytarowski
>>>
>>> Patch By: Manikishan
>>>
>>> Tags: #clang, #clang-format
>>>
>>> Differential Revision: https://protect2.fireeye.com/url?k=94716b7f-c8a560c1-94712be4-86742d02e7e2-97a1f15f4847e953&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD64695
>>>
>>> Modified:
>>> cfe/trunk/docs/ClangFormatStyleOptions.rst
>>> cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
>>> cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
>>> cfe/trunk/lib/Format/Format.cpp
>>> cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
>>> cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
>>> cfe/trunk/unittests/Format/SortIncludesTest.cpp
>>>
>>> Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
>>> URL: https://protect2.fireeye.com/url?k=d1dd6b38-8d096086-d1dd2ba3-86742d02e7e2-0c39641cd0846c27&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Fdocs%2FClangFormatStyleOptions.rst%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
>>> +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Sep 25 13:33:01 2019
>>> @@ -1511,6 +1511,13 @@ the configuration (without a prefix: ``A
>>> can also assign negative priorities if you have certain headers that
>>> always need to be first.
>>>
>>> + There is a third and optional field ``SortPriority`` which can used while
>>> + ``IncludeBloks = IBS_Regroup`` to define the priority in which ``#includes``
>>> + should be ordered, and value of ``Priority`` defines the order of
>>> + ``#include blocks`` and also enables to group ``#includes`` of different
>>> + priority for order.``SortPriority`` is set to the value of ``Priority``
>>> + as default if it is not assigned.
>>> +
>>> To configure this in the .clang-format file, use:
>>>
>>> .. code-block:: yaml
>>> @@ -1518,12 +1525,14 @@ the configuration (without a prefix: ``A
>>> IncludeCategories:
>>> - Regex: '^"(llvm|llvm-c|clang|clang-c)/'
>>> Priority: 2
>>> + SortPriority: 2
>>> - Regex: '^(<|"(gtest|gmock|isl|json)/)'
>>> Priority: 3
>>> - Regex: '<[[:alnum:].]+>'
>>> Priority: 4
>>> - Regex: '.*'
>>> Priority: 1
>>> + SortPriority: 0
>>>
>>> **IncludeIsMainRegex** (``std::string``)
>>> Specify a regular expression of suffixes that are allowed in the
>>>
>>> Modified: cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h
>>> URL: https://protect2.fireeye.com/url?k=a3d4ca4f-ff00c1f1-a3d48ad4-86742d02e7e2-93a045b7836bde46&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Finclude%2Fclang%2FTooling%2FInclusions%2FHeaderIncludes.h%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h (original)
>>> +++ cfe/trunk/include/clang/Tooling/Inclusions/HeaderIncludes.h Wed Sep 25 13:33:01 2019
>>> @@ -32,6 +32,7 @@ public:
>>> /// 0. Otherwise, returns the priority of the matching category or INT_MAX.
>>> /// NOTE: this API is not thread-safe!
>>> int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const;
>>> + int getSortIncludePriority(StringRef IncludeName, bool CheckMainHeader) const;
>>>
>>> private:
>>> bool isMainHeader(StringRef IncludeName) const;
>>>
>>> Modified: cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h
>>> URL: https://protect2.fireeye.com/url?k=5058cb1a-0c8cc0a4-50588b81-86742d02e7e2-1af614b360b8f2fb&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Finclude%2Fclang%2FTooling%2FInclusions%2FIncludeStyle.h%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h (original)
>>> +++ cfe/trunk/include/clang/Tooling/Inclusions/IncludeStyle.h Wed Sep 25 13:33:01 2019
>>> @@ -58,6 +58,8 @@ struct IncludeStyle {
>>> std::string Regex;
>>> /// The priority to assign to this category.
>>> int Priority;
>>> + /// The custom priority to sort before grouping.
>>> + int SortPriority;
>>> bool operator==(const IncludeCategory &Other) const {
>>> return Regex == Other.Regex && Priority == Other.Priority;
>>> }
>>>
>>> Modified: cfe/trunk/lib/Format/Format.cpp
>>> URL: https://protect2.fireeye.com/url?k=321df2b5-6ec9f90b-321db22e-86742d02e7e2-8c5ee43df70917f7&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FFormat%2FFormat.cpp%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Format/Format.cpp (original)
>>> +++ cfe/trunk/lib/Format/Format.cpp Wed Sep 25 13:33:01 2019
>>> @@ -1771,6 +1771,7 @@ struct IncludeDirective {
>>> StringRef Text;
>>> unsigned Offset;
>>> int Category;
>>> + int Priority;
>>> };
>>>
>>> struct JavaImportDirective {
>>> @@ -1834,6 +1835,7 @@ static void sortCppIncludes(const Format
>>> ArrayRef<tooling::Range> Ranges, StringRef FileName,
>>> StringRef Code, tooling::Replacements &Replaces,
>>> unsigned *Cursor) {
>>> + tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
>>> unsigned IncludesBeginOffset = Includes.front().Offset;
>>> unsigned IncludesEndOffset =
>>> Includes.back().Offset + Includes.back().Text.size();
>>> @@ -1841,11 +1843,12 @@ static void sortCppIncludes(const Format
>>> if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
>>> return;
>>> SmallVector<unsigned, 16> Indices;
>>> - for (unsigned i = 0, e = Includes.size(); i != e; ++i)
>>> + for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
>>> Indices.push_back(i);
>>> + }
>>> llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
>>> - return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
>>> - std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
>>> + return std::tie(Includes[LHSI].Priority, Includes[LHSI].Filename) <
>>> + std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename);
>>> });
>>> // The index of the include on which the cursor will be put after
>>> // sorting/deduplicating.
>>> @@ -1960,9 +1963,12 @@ tooling::Replacements sortCppIncludes(co
>>> int Category = Categories.getIncludePriority(
>>> IncludeName,
>>> /*CheckMainHeader=*/!MainIncludeFound && FirstIncludeBlock);
>>> + int Priority = Categories.getSortIncludePriority(
>>> + IncludeName, !MainIncludeFound && FirstIncludeBlock);
>>> if (Category == 0)
>>> MainIncludeFound = true;
>>> - IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
>>> + IncludesInBlock.push_back(
>>> + {IncludeName, Line, Prev, Category, Priority});
>>> } else if (!IncludesInBlock.empty() && !EmptyLineSkipped) {
>>> sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Code,
>>> Replaces, Cursor);
>>>
>>> Modified: cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp
>>> URL: https://protect2.fireeye.com/url?k=adb58601-f1618dbf-adb5c69a-86742d02e7e2-532943b9f04d415c&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FTooling%2FInclusions%2FHeaderIncludes.cpp%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp (original)
>>> +++ cfe/trunk/lib/Tooling/Inclusions/HeaderIncludes.cpp Wed Sep 25 13:33:01 2019
>>> @@ -199,6 +199,20 @@ int IncludeCategoryManager::getIncludePr
>>> return Ret;
>>> }
>>>
>>> +int IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName,
>>> + bool CheckMainHeader) const {
>>> + int Ret = INT_MAX;
>>> + for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i)
>>> + if (CategoryRegexs[i].match(IncludeName)) {
>>> + Ret = Style.IncludeCategories[i].SortPriority;
>>> + if (Ret == 0)
>>> + Ret = Style.IncludeCategories[i].Priority;
>>> + break;
>>> + }
>>> + if (CheckMainHeader && IsMainFile && Ret > 0 && isMainHeader(IncludeName))
>>> + Ret = 0;
>>> + return Ret;
>>> +}
>>> bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const {
>>> if (!IncludeName.startswith("\""))
>>> return false;
>>>
>>> Modified: cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp
>>> URL: https://protect2.fireeye.com/url?k=4cf5fe3c-1021f582-4cf5bea7-86742d02e7e2-3cf9be10b6faae32&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FTooling%2FInclusions%2FIncludeStyle.cpp%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp (original)
>>> +++ cfe/trunk/lib/Tooling/Inclusions/IncludeStyle.cpp Wed Sep 25 13:33:01 2019
>>> @@ -17,6 +17,7 @@ void MappingTraits<IncludeStyle::Include
>>> IO &IO, IncludeStyle::IncludeCategory &Category) {
>>> IO.mapOptional("Regex", Category.Regex);
>>> IO.mapOptional("Priority", Category.Priority);
>>> + IO.mapOptional("SortPriority", Category.SortPriority);
>>> }
>>>
>>> void ScalarEnumerationTraits<IncludeStyle::IncludeBlocksStyle>::enumeration(
>>>
>>> Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp
>>> URL: https://protect2.fireeye.com/url?k=d39f8412-8f4b8fac-d39fc489-86742d02e7e2-02cdbe423900a25c&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Funittests%2FFormat%2FSortIncludesTest.cpp%3Frev%3D372919%26r1%3D372918%26r2%3D372919%26view%3Ddiff
>>> ==============================================================================
>>> --- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)
>>> +++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Wed Sep 25 13:33:01 2019
>>> @@ -70,6 +70,77 @@ TEST_F(SortIncludesTest, BasicSorting) {
>>> {tooling::Range(25, 1)}));
>>> }
>>>
>>> +TEST_F(SortIncludesTest, SortedIncludesUsingSortPriorityAttribute) {
>>> + FmtStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
>>> + FmtStyle.IncludeStyle.IncludeCategories = {
>>> + {"^<sys/param\\.h>", 1, 0},
>>> + {"^<sys/types\\.h>", 1, 1},
>>> + {"^<sys.*/", 1, 2},
>>> + {"^<uvm/", 2, 3},
>>> + {"^<machine/", 3, 4},
>>> + {"^<dev/", 4, 5},
>>> + {"^<net.*/", 5, 6},
>>> + {"^<protocols/", 5, 7},
>>> + {"^<(fs|miscfs|msdosfs|nfs|ntfs|ufs)/", 6, 8},
>>> + {"^<(x86|amd64|i386|xen)/", 7, 8},
>>> + {"<path", 9, 11},
>>> + {"^<[^/].*\\.h>", 8, 10},
>>> + {"^\".*\\.h\"", 10, 12}};
>>> + EXPECT_EQ("#include <sys/param.h>\n"
>>> + "#include <sys/types.h>\n"
>>> + "#include <sys/ioctl.h>\n"
>>> + "#include <sys/socket.h>\n"
>>> + "#include <sys/stat.h>\n"
>>> + "#include <sys/wait.h>\n"
>>> + "\n"
>>> + "#include <net/if.h>\n"
>>> + "#include <net/if_dl.h>\n"
>>> + "#include <net/route.h>\n"
>>> + "#include <netinet/in.h>\n"
>>> + "#include <protocols/rwhod.h>\n"
>>> + "\n"
>>> + "#include <assert.h>\n"
>>> + "#include <errno.h>\n"
>>> + "#include <inttypes.h>\n"
>>> + "#include <stdio.h>\n"
>>> + "#include <stdlib.h>\n"
>>> + "\n"
>>> + "#include <paths.h>\n"
>>> + "\n"
>>> + "#include \"pathnames.h\"\n",
>>> + sort("#include <sys/param.h>\n"
>>> + "#include <sys/types.h>\n"
>>> + "#include <sys/ioctl.h>\n"
>>> + "#include <net/if_dl.h>\n"
>>> + "#include <net/route.h>\n"
>>> + "#include <netinet/in.h>\n"
>>> + "#include <sys/socket.h>\n"
>>> + "#include <sys/stat.h>\n"
>>> + "#include <sys/wait.h>\n"
>>> + "#include <net/if.h>\n"
>>> + "#include <protocols/rwhod.h>\n"
>>> + "#include <assert.h>\n"
>>> + "#include <paths.h>\n"
>>> + "#include \"pathnames.h\"\n"
>>> + "#include <errno.h>\n"
>>> + "#include <inttypes.h>\n"
>>> + "#include <stdio.h>\n"
>>> + "#include <stdlib.h>\n"));
>>> +}
>>> +TEST_F(SortIncludesTest, SortPriorityNotDefined) {
>>> + FmtStyle = getLLVMStyle();
>>> + EXPECT_EQ("#include \"FormatTestUtils.h\"\n"
>>> + "#include \"clang/Format/Format.h\"\n"
>>> + "#include \"llvm/ADT/None.h\"\n"
>>> + "#include \"llvm/Support/Debug.h\"\n"
>>> + "#include \"gtest/gtest.h\"\n",
>>> + sort("#include \"clang/Format/Format.h\"\n"
>>> + "#include \"llvm/ADT/None.h\"\n"
>>> + "#include \"FormatTestUtils.h\"\n"
>>> + "#include \"gtest/gtest.h\"\n"
>>> + "#include \"llvm/Support/Debug.h\"\n"));
>>> +}
>>> +
>>> TEST_F(SortIncludesTest, NoReplacementsForValidIncludes) {
>>> // Identical #includes have led to a failure with an unstable sort.
>>> std::string Code = "#include <a>\n"
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> https://protect2.fireeye.com/url?k=44143b0a-18c030b4-44147b91-86742d02e7e2-45a48425fd884b69&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://protect2.fireeye.com/url?k=0b029229-57d69997-0b02d2b2-86742d02e7e2-467fd19ff420a131&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://protect2.fireeye.com/url?k=44224080-18f150e8-4422001b-8691b328a8b8-8d51a0fcc2d8017d&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits
>
More information about the cfe-commits
mailing list