diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1376,9 +1376,9 @@ LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH"); LLVMStyle.IfMacros.push_back("KJ_IF_MAYBE"); LLVMStyle.IncludeStyle.IncludeCategories = { - {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, - {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false}, - {".*", 1, 0, false}}; + {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 2, false}, + {"^(<|\"(gtest|gmock|isl|json)/)", 3, 3, false}, + {".*", 1, 1, false}}; LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$"; LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve; LLVMStyle.IndentAccessModifiers = false; @@ -1506,10 +1506,10 @@ GoogleStyle.AlwaysBreakBeforeMultilineStrings = true; GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes; GoogleStyle.DerivePointerAlignment = true; - GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 0, false}, - {"^<.*\\.h>", 1, 0, false}, - {"^<.*", 2, 0, false}, - {".*", 3, 0, false}}; + GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 2, false}, + {"^<.*\\.h>", 1, 1, false}, + {"^<.*", 2, 2, false}, + {".*", 3, 3, false}}; GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$"; GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup; GoogleStyle.IndentCaseLabels = true; @@ -2966,16 +2966,12 @@ SmallVector Matches; SmallVector IncludesInBlock; - // In compiled files, consider the first #include to be the main #include of - // the file if it is not a system #include. This ensures that the header - // doesn't have hidden dependencies - // (http://llvm.org/docs/CodingStandards.html#include-style). - // // FIXME: Do some validation, 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; + // Todo(remove before merge): removed: multiple files can be main includes + // This option caused random sorting, depending which was detected first. bool FormattingOff = false; // '[' must be the first and '-' the last character inside [...]. @@ -3029,11 +3025,11 @@ } int Category = Categories.getIncludePriority( IncludeName, - /*CheckMainHeader=*/!MainIncludeFound && FirstIncludeBlock); - int Priority = Categories.getSortIncludePriority( - IncludeName, !MainIncludeFound && FirstIncludeBlock); - if (Category == 0) - MainIncludeFound = true; + /*CheckMainHeader=*/FirstIncludeBlock); + int Priority = + Categories.getSortIncludePriority(IncludeName, FirstIncludeBlock); + // Todo(remove before merge): reason for removal: every header can have + // 0,0 priority IncludesInBlock.push_back( {IncludeName, Line, Prev, Category, Priority}); } else if (!IncludesInBlock.empty() && !EmptyLineSkipped) { diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp --- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp +++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp @@ -206,33 +206,35 @@ } } +constexpr int DefaultMainIncludePriority = 0; +constexpr int DefaultMainIncludeSortPriority = 0; + int IncludeCategoryManager::getIncludePriority(StringRef IncludeName, bool CheckMainHeader) const { - int Ret = INT_MAX; + if (CheckMainHeader && IsMainFile && isMainHeader(IncludeName)) + return DefaultMainIncludePriority; + for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i) if (CategoryRegexs[i].match(IncludeName)) { - Ret = Style.IncludeCategories[i].Priority; - break; + return Style.IncludeCategories[i].Priority; } - if (CheckMainHeader && IsMainFile && Ret > 0 && isMainHeader(IncludeName)) - Ret = 0; - return Ret; + + return std::numeric_limits::max(); } int IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName, bool CheckMainHeader) const { - int Ret = INT_MAX; + if (CheckMainHeader && IsMainFile && isMainHeader(IncludeName)) + return DefaultMainIncludeSortPriority; + 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; + return Style.IncludeCategories[i].SortPriority; } - if (CheckMainHeader && IsMainFile && Ret > 0 && isMainHeader(IncludeName)) - Ret = 0; - return Ret; + + return std::numeric_limits::max(); } + bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const { if (!IncludeName.startswith("\"")) return false; diff --git a/clang/lib/Tooling/Inclusions/IncludeStyle.cpp b/clang/lib/Tooling/Inclusions/IncludeStyle.cpp --- a/clang/lib/Tooling/Inclusions/IncludeStyle.cpp +++ b/clang/lib/Tooling/Inclusions/IncludeStyle.cpp @@ -15,9 +15,9 @@ void MappingTraits::mapping( IO &IO, IncludeStyle::IncludeCategory &Category) { - IO.mapOptional("Regex", Category.Regex); - IO.mapOptional("Priority", Category.Priority); - IO.mapOptional("SortPriority", Category.SortPriority); + IO.mapRequired("Regex", Category.Regex); + IO.mapRequired("Priority", Category.Priority); + IO.mapOptional("SortPriority", Category.SortPriority, Category.Priority); IO.mapOptional("CaseSensitive", Category.RegexIsCaseSensitive); } diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp --- a/clang/unittests/Format/SortIncludesTest.cpp +++ b/clang/unittests/Format/SortIncludesTest.cpp @@ -87,7 +87,7 @@ TEST_F(SortIncludesTest, SortedIncludesUsingSortPriorityAttribute) { FmtStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup; FmtStyle.IncludeStyle.IncludeCategories = { - {"^", 1, 0, false}, + {"^", 1, 1, false}, {"^", 1, 1, false}, {"^ a.priv.h && a.h + // E.g. a_test.cpp -> a_test.h && a.h + // Maybe add a "//clang-format pragma: not_main" to remove false positives + + // Recognize all possible main #include's with a matching basename as main + // include. EXPECT_EQ("#include \"a.h\"\n" + "#include \"llvm/a.h\"\n" "#include \"b.h\"\n" - "#include \"c.h\"\n" - "#include \"llvm/a.h\"\n", + "#include \"c.h\"\n", sort("#include \"b.h\"\n" "#include \"a.h\"\n" "#include \"c.h\"\n" @@ -645,8 +652,9 @@ "a.h")); Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup; - Style.IncludeCategories = { - {"^\"", 1, 0, false}, {"^<.*\\.h>$", 2, 0, false}, {"^<", 3, 0, false}}; + Style.IncludeCategories = {{"^\"", 1, 1, false}, // + {"^<.*\\.h>$", 2, 2, false}, + {"^<", 3, 3, false}}; StringRef UnsortedCode = "#include \"qt.h\"\n" "#include \n" @@ -695,11 +703,11 @@ TEST_F(SortIncludesTest, SupportOptionalCaseSensitiveMachting) { Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup; - Style.IncludeCategories = {{"^\"", 1, 0, false}, - {"^<.*\\.h>$", 2, 0, false}, - {"^", 3, 0, false}, - {"^", 4, 0, false}, - {"^<", 5, 0, false}}; + Style.IncludeCategories = {{"^\"", 1, 1, false}, + {"^<.*\\.h>$", 2, 2, false}, + {"^", 3, 3, false}, + {"^", 4, 4, false}, + {"^<", 5, 5, false}}; StringRef UnsortedCode = "#include \n" "#include \"qt.h\"\n" @@ -744,8 +752,8 @@ } TEST_F(SortIncludesTest, NegativePriorities) { - Style.IncludeCategories = {{".*important_os_header.*", -1, 0, false}, - {".*", 1, 0, false}}; + Style.IncludeCategories = {{".*important_os_header.*", -1, -1, false}, + {".*", 1, 1, false}}; EXPECT_EQ("#include \"important_os_header.h\"\n" "#include \"c_main.h\"\n" "#include \"a_other.h\"\n", @@ -765,8 +773,8 @@ } TEST_F(SortIncludesTest, PriorityGroupsAreSeparatedWhenRegroupping) { - Style.IncludeCategories = {{".*important_os_header.*", -1, 0, false}, - {".*", 1, 0, false}}; + Style.IncludeCategories = {{".*important_os_header.*", -1, -1, false}, + {".*", 1, 1, false}}; Style.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup; EXPECT_EQ("#include \"important_os_header.h\"\n"