kentsommer updated this revision to Diff 318886.
kentsommer marked 5 inline comments as done.
kentsommer added a comment.
Addressed comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95017/new/
https://reviews.llvm.org/D95017
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Tooling/Inclusions/IncludeStyle.h
clang/lib/Format/Format.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortIncludesTest.cpp
Index: clang/unittests/Format/SortIncludesTest.cpp
===================================================================
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -598,6 +598,49 @@
"a.cc"));
}
+TEST_F(SortIncludesTest, SupportOptionalAlphabeticalSorting) {
+ EXPECT_FALSE(Style.IncludeSortAlphabetically);
+
+ Style.IncludeSortAlphabetically = true;
+
+ EXPECT_EQ("#include \"A/B.h\"\n"
+ "#include \"A/b.h\"\n"
+ "#include \"a/b.h\"\n"
+ "#include \"B/A.h\"\n"
+ "#include \"B/a.h\"\n",
+ sort("#include \"B/a.h\"\n"
+ "#include \"B/A.h\"\n"
+ "#include \"A/B.h\"\n"
+ "#include \"a/b.h\"\n"
+ "#include \"A/b.h\"\n",
+ "a.h"));
+
+ Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup;
+ Style.IncludeCategories = {
+ {"^\"", 1, 0, false}, {"^<.*\\.h>$", 2, 0, false}, {"^<", 3, 0, false}};
+
+ StringRef UnsortedCode = "#include \"qt.h\"\n"
+ "#include <algorithm>\n"
+ "#include <qtwhatever.h>\n"
+ "#include <Qtwhatever.h>\n"
+ "#include <Algorithm>\n"
+ "#include \"vlib.h\"\n"
+ "#include \"Vlib.h\"\n"
+ "#include \"AST.h\"\n";
+
+ EXPECT_EQ("#include \"AST.h\"\n"
+ "#include \"qt.h\"\n"
+ "#include \"Vlib.h\"\n"
+ "#include \"vlib.h\"\n"
+ "\n"
+ "#include <Qtwhatever.h>\n"
+ "#include <qtwhatever.h>\n"
+ "\n"
+ "#include <Algorithm>\n"
+ "#include <algorithm>\n",
+ sort(UnsortedCode));
+}
+
TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
// Setup an regex for main includes so we can cover those as well.
Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14395,6 +14395,8 @@
CHECK_PARSE_BOOL(DeriveLineEnding);
CHECK_PARSE_BOOL(DerivePointerAlignment);
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
+ CHECK_PARSE_BOOL_FIELD(IncludeStyle.IncludeSortAlphabetically,
+ "IncludeSortAlphabetically");
CHECK_PARSE_BOOL(DisableFormat);
CHECK_PARSE_BOOL(IndentCaseLabels);
CHECK_PARSE_BOOL(IndentCaseBlocks);
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -556,6 +556,8 @@
IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
IO.mapOptional("IncludeIsMainSourceRegex",
Style.IncludeStyle.IncludeIsMainSourceRegex);
+ IO.mapOptional("IncludeSortAlphabetically",
+ Style.IncludeStyle.IncludeSortAlphabetically);
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
@@ -925,6 +927,7 @@
{".*", 1, 0, false}};
LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
+ LLVMStyle.IncludeStyle.IncludeSortAlphabetically = false;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentCaseBlocks = false;
LLVMStyle.IndentGotoLabels = true;
@@ -2179,10 +2182,23 @@
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].Priority, Includes[LHSI].Filename) <
- std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename);
- });
+
+ if (Style.IncludeStyle.IncludeSortAlphabetically) {
+ llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
+ const auto LHSFilenameLower = Includes[LHSI].Filename.lower();
+ const auto RHSFilenameLower = Includes[RHSI].Filename.lower();
+ return std::tie(Includes[LHSI].Priority, LHSFilenameLower,
+ Includes[LHSI].Filename) <
+ std::tie(Includes[RHSI].Priority, RHSFilenameLower,
+ Includes[RHSI].Filename);
+ });
+ } else {
+ llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
+ 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.
unsigned CursorIndex;
@@ -2268,8 +2284,8 @@
// doesn't have hidden dependencies
// (http://llvm.org/docs/CodingStandards.html#include-style).
//
- // FIXME: Do some sanity checking, e.g. edit distance of the base name, to fix
- // cases where the first #include is unlikely to be the main header.
+ // FIXME: Do some sanity checking, e.g. edit distance of the base name, to
+ // fix cases where the first #include is unlikely to be the main header.
tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
bool FirstIncludeBlock = true;
bool MainIncludeFound = false;
@@ -2330,8 +2346,8 @@
return Replaces;
}
-// Returns group number to use as a first order sort on imports. Gives UINT_MAX
-// if the import does not match any given groups.
+// Returns group number to use as a first order sort on imports. Gives
+// UINT_MAX if the import does not match any given groups.
static unsigned findJavaImportGroup(const FormatStyle &Style,
StringRef ImportIdentifier) {
unsigned LongestMatchIndex = UINT_MAX;
@@ -2581,8 +2597,8 @@
tooling::Replacements Result;
for (const auto &R : Replaces) {
if (isHeaderInsertion(R)) {
- // Replacements from \p Replaces must be conflict-free already, so we can
- // simply consume the error.
+ // Replacements from \p Replaces must be conflict-free already, so we
+ // can simply consume the error.
llvm::consumeError(HeaderInsertions.add(R));
} else if (isHeaderDeletion(R)) {
HeadersToDelete.insert(R.getReplacementText());
@@ -2652,7 +2668,8 @@
StringRef FileName) -> tooling::Replacements {
return cleanup(Style, Code, Ranges, FileName);
};
- // Make header insertion replacements insert new headers into correct blocks.
+ // Make header insertion replacements insert new headers into correct
+ // blocks.
tooling::Replacements NewReplaces =
fixCppIncludeInsertions(Code, Replaces, Style);
return processReplacements(Cleanup, Code, NewReplaces, Style);
@@ -2844,8 +2861,8 @@
const auto GuessedLanguage = getLanguageByFileName(FileName);
if (GuessedLanguage == FormatStyle::LK_Cpp) {
auto Extension = llvm::sys::path::extension(FileName);
- // If there's no file extension (or it's .h), we need to check the contents
- // of the code to see if it contains Objective-C.
+ // If there's no file extension (or it's .h), we need to check the
+ // contents of the code to see if it contains Objective-C.
if (Extension.empty() || Extension == ".h") {
auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
Environment Env(Code, NonEmptyFileName, /*Ranges=*/{});
@@ -2890,7 +2907,8 @@
return Style;
}
- // Look for .clang-format/_clang-format file in the file's parent directories.
+ // Look for .clang-format/_clang-format file in the file's parent
+ // directories.
SmallString<128> UnsuitableConfigFiles;
SmallString<128> Path(FileName);
if (std::error_code EC = FS->makeAbsolute(Path))
Index: clang/include/clang/Tooling/Inclusions/IncludeStyle.h
===================================================================
--- clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -147,6 +147,26 @@
/// ``ClassImpl.hpp`` would not have the main include file put on top
/// before any other include.
std::string IncludeIsMainSourceRegex;
+
+ /// Specify if sorting should be done in an alphabetical and
+ /// case sensitive fashion.
+ ///
+ /// When ``false``, includes are sorted in an ASCIIbetical
+ /// fashion.
+ /// When ``true``, includes are sorted in an alphabetical
+ /// fashion with case used as a tie-breaker.
+ ///
+ /// \code
+ /// false: true:
+ /// #include "A/B.h" vs. #include "A/B.h"
+ /// #include "A/b.h" #include "A/b.h"
+ /// #include "B/A.h" #include "a/b.h"
+ /// #include "B/a.h" #include "B/A.h"
+ /// #include "a/b.h" #include "B/a.h"
+ /// \endcode
+ ///
+ /// This option is off by default.
+ bool IncludeSortAlphabetically;
};
} // namespace tooling
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2036,6 +2036,26 @@
``ClassImpl.hpp`` would not have the main include file put on top
before any other include.
+**IncludeSortAlphabetically** (``bool``)
+ Specify if sorting should be done in an alphabetical and
+ case sensitive fashion.
+
+ When ``false``, includes are sorted in an ASCIIbetical
+ fashion.
+ When ``true``, includes are sorted in an alphabetical
+ fashion with case used as a tie-breaker.
+
+ .. code-block:: c++
+
+ false: true:
+ #include "A/B.h" vs. #include "A/B.h"
+ #include "A/b.h" #include "A/b.h"
+ #include "B/A.h" #include "a/b.h"
+ #include "B/a.h" #include "B/A.h"
+ #include "a/b.h" #include "B/a.h"
+
+ This option is off by default.
+
**IndentCaseBlocks** (``bool``)
Indent case label blocks one level from the case label.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
