<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 19, 2015 at 4:24 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hm, seems to me that this is broken either way. If config.h remains first, that is good, but the main #include is unlikely to remain second.</div></blockquote><div><br></div><div>In practice, the first two lines are a block with two includes: config.h first and the main header seconds. Since clang-format doesn't move the first line around and since it does include-block-internal sorting only, it currently never reorders this first two-includes block.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> I think we should give the main #include a non-zero #include category and then properly configure config.h to be before that. I'll try to get to that this week.</div></blockquote><div><br></div><div>Sounds great, thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Oct 18, 2015 at 6:40 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Tue, Sep 29, 2015 at 12:53 AM, Daniel Jasper via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: djasper<br>
Date: Tue Sep 29 02:53:08 2015<br>
New Revision: 248782<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248782&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=248782&view=rev</a><br>
Log:<br>
clang-format: Extend #include sorting functionality<br>
<br>
Recognize the main module header as well as different #include categories.<br>
This should now mimic the behavior of llvm/utils/sort_includes.py as<br>
well as clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp very<br>
closely.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Format/Format.h<br>
cfe/trunk/lib/Format/Format.cpp<br>
cfe/trunk/tools/clang-format/ClangFormat.cpp<br>
cfe/trunk/unittests/Format/SortIncludesTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Format/Format.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=248782&r1=248781&r2=248782&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=248782&r1=248781&r2=248782&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Format/Format.h (original)<br>
+++ cfe/trunk/include/clang/Format/Format.h Tue Sep 29 02:53:08 2015<br>
@@ -259,6 +259,21 @@ struct FormatStyle {<br>
/// For example: BOOST_FOREACH.<br>
std::vector<std::string> ForEachMacros;<br>
<br>
+ /// \brief Regular expressions denoting the different #include categories used<br>
+ /// for ordering #includes.<br>
+ ///<br>
+ /// These regular expressions are matched against the filename of an include<br>
+ /// (including the <> or "") in order. The value belonging to the first<br>
+ /// matching regular expression is assigned and #includes are sorted first<br>
+ /// according to increasing category number and then alphabetically within<br>
+ /// each category.<br>
+ ///<br>
+ /// If none of the regular expressions match, UINT_MAX is assigned as<br>
+ /// category. The main header for a source file automatically gets category 0,<br>
+ /// so that it is kept at the beginning of the #includes<br>
+ /// (<a href="http://llvm.org/docs/CodingStandards.html#include-style" rel="noreferrer" target="_blank">http://llvm.org/docs/CodingStandards.html#include-style</a>).<br>
+ std::vector<std::pair<std::string, unsigned>> IncludeCategories;<br>
+<br>
/// \brief Indent case labels one level from the switch statement.<br>
///<br>
/// When \c false, use the same indentation level as for the switch statement.<br>
<br>
Modified: cfe/trunk/lib/Format/Format.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=248782&r1=248781&r2=248782&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=248782&r1=248781&r2=248782&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Tue Sep 29 02:53:08 2015<br>
@@ -13,6 +13,7 @@<br>
///<br>
//===----------------------------------------------------------------------===//<br>
<br>
+#include "clang/Format/Format.h"<br>
#include "ContinuationIndenter.h"<br>
#include "TokenAnnotator.h"<br>
#include "UnwrappedLineFormatter.h"<br>
@@ -21,7 +22,6 @@<br>
#include "clang/Basic/Diagnostic.h"<br>
#include "clang/Basic/DiagnosticOptions.h"<br>
#include "clang/Basic/SourceManager.h"<br>
-#include "clang/Format/Format.h"<br>
#include "clang/Lex/Lexer.h"<br>
#include "llvm/ADT/STLExtras.h"<br>
#include "llvm/Support/Allocator.h"<br>
@@ -375,6 +375,9 @@ FormatStyle getLLVMStyle() {<br>
LLVMStyle.ForEachMacros.push_back("foreach");<br>
LLVMStyle.ForEachMacros.push_back("Q_FOREACH");<br>
LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");<br>
+ LLVMStyle.IncludeCategories = {{"^\"(llvm|llvm-c|clang|clang-c)/", 2},<br>
+ {"^(<|\"(gtest|isl|json)/)", 3},<br>
+ {".*", 1}};<br>
LLVMStyle.IndentCaseLabels = false;<br>
LLVMStyle.IndentWrappedFunctionNames = false;<br>
LLVMStyle.IndentWidth = 2;<br>
@@ -423,6 +426,7 @@ FormatStyle getGoogleStyle(FormatStyle::<br>
GoogleStyle.AlwaysBreakTemplateDeclarations = true;<br>
GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;<br>
GoogleStyle.DerivePointerAlignment = true;<br>
+ GoogleStyle.IncludeCategories = {{"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};<br>
GoogleStyle.IndentCaseLabels = true;<br>
GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;<br>
GoogleStyle.ObjCSpaceAfterProperty = false;<br>
@@ -1575,7 +1579,7 @@ struct IncludeDirective {<br>
StringRef Filename;<br>
StringRef Text;<br>
unsigned Offset;<br>
- bool IsAngled;<br>
+ unsigned Category;<br>
};<br>
<br>
} // end anonymous namespace<br>
@@ -1605,7 +1609,8 @@ static void sortIncludes(const FormatSty<br>
for (unsigned i = 0, e = Includes.size(); i != e; ++i)<br>
Indices.push_back(i);<br>
std::sort(Indices.begin(), Indices.end(), [&](unsigned LHSI, unsigned RHSI) {<br>
- return Includes[LHSI].Filename < Includes[RHSI].Filename;<br>
+ return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <<br>
+ std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);<br>
});<br>
<br>
// If the #includes are out of order, we generate a single replacement fixing<br>
@@ -<a href="tel:1642" value="+491642" target="_blank">1642</a>,22 +1647,49 @@ tooling::Replacements sortIncludes(const<br>
tooling::Replacements Replaces;<br>
unsigned Prev = 0;<br>
unsigned SearchFrom = 0;<br>
- llvm::Regex IncludeRegex(R"(^[\t\ ]*#[\t\ ]*include[^"<]*["<]([^">]*)([">]))");<br>
+ llvm::Regex IncludeRegex(<br>
+ R"(^[\t\ ]*#[\t\ ]*include[^"<]*(["<][^">]*[">]))");<br>
SmallVector<StringRef, 4> Matches;<br>
SmallVector<IncludeDirective, 16> IncludesInBlock;<br>
+<br>
+ // In compiled files, consider the first #include to be the main #include of<br>
+ // the file if it is not a system #include. This ensures that the header<br>
+ // doesn't have hidden dependencies<br>
+ // (<a href="http://llvm.org/docs/CodingStandards.html#include-style" rel="noreferrer" target="_blank">http://llvm.org/docs/CodingStandards.html#include-style</a>).<br>
+ //<br>
+ // FIXME: Do some sanity checking, e.g. edit distance of the base name, to fix<br>
+ // cases where the first #include is unlikely to be the main header.<br></blockquote><div><br></div></div></div><div>FYI: This happens to make include sorting in webkit cpp files work too, where the style guide says that one must include config.h first and the main header second (<a href="https://www.webkit.org/coding/coding-style.html" target="_blank">https://www.webkit.org/coding/coding-style.html</a>). Since clang-format won't reorder the first line, it keeps the config.h header first, even though config.h isn't technically the main header. If this FIXME gets fixed, it'd be good if formatting of webkit cpp files isn't broke in the process.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ bool LookForMainHeader = FileName.endswith(".c") ||<br>
+ FileName.endswith(".cc") ||<br>
+ FileName.endswith(".cpp")||<br>
+ FileName.endswith(".c++")||<br>
+ FileName.endswith(".cxx");<br>
+<br>
+ // Create pre-compiled regular expressions for the #include categories.<br>
+ SmallVector<llvm::Regex, 4> CategoryRegexs;<br>
+ for (const auto &IncludeBlock : Style.IncludeCategories)<br>
+ CategoryRegexs.emplace_back(IncludeBlock.first);<br>
+<br>
for (;;) {<br>
auto Pos = Code.find('\n', SearchFrom);<br>
StringRef Line =<br>
Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);<br>
if (!Line.endswith("\\")) {<br>
if (IncludeRegex.match(Line, &Matches)) {<br>
- bool IsAngled = Matches[2] == ">";<br>
- if (!IncludesInBlock.empty() &&<br>
- IsAngled != IncludesInBlock.back().IsAngled) {<br>
- sortIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces);<br>
- IncludesInBlock.clear();<br>
+ unsigned Category;<br>
+ if (LookForMainHeader && !Matches[1].startswith("<")) {<br>
+ Category = 0;<br>
+ } else {<br>
+ Category = UINT_MAX;<br>
+ for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i) {<br>
+ if (CategoryRegexs[i].match(Matches[1])) {<br>
+ Category = Style.IncludeCategories[i].second;<br>
+ break;<br>
+ }<br>
+ }<br>
}<br>
- IncludesInBlock.push_back({Matches[1], Line, Prev, Matches[2] == ">"});<br>
+ LookForMainHeader = false;<br>
+ IncludesInBlock.push_back({Matches[1], Line, Prev, Category});<br>
} else if (!IncludesInBlock.empty()) {<br>
sortIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces);<br>
IncludesInBlock.clear();<br>
<br>
Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=248782&r1=248781&r2=248782&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=248782&r1=248781&r2=248782&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)<br>
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Tue Sep 29 02:53:08 2015<br>
@@ -73,7 +73,7 @@ FallbackStyle("fallback-style",<br>
cl::init("LLVM"), cl::cat(ClangFormatCategory));<br>
<br>
static cl::opt<std::string><br>
-AssumeFilename("assume-filename",<br>
+AssumeFileName("assume-filename",<br>
cl::desc("When reading from stdin, clang-format assumes this\n"<br>
"filename to look for a style config file (with\n"<br>
"-style=file) and to determine the language."),<br>
@@ -239,13 +239,13 @@ static bool format(StringRef FileName) {<br>
std::vector<tooling::Range> Ranges;<br>
if (fillRanges(Code.get(), Ranges))<br>
return true;<br>
- FormatStyle FormatStyle = getStyle(<br>
- Style, (FileName == "-") ? AssumeFilename : FileName, FallbackStyle);<br>
+ StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName;<br>
+ FormatStyle FormatStyle = getStyle(Style, AssumedFileName, FallbackStyle);<br>
Replacements Replaces;<br>
std::string ChangedCode;<br>
if (SortIncludes) {<br>
Replaces =<br>
- sortIncludes(FormatStyle, Code->getBuffer(), Ranges, FileName);<br>
+ sortIncludes(FormatStyle, Code->getBuffer(), Ranges, AssumedFileName);<br>
ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);<br>
for (const auto &R : Replaces)<br>
Ranges.push_back({R.getOffset(), R.getLength()});<br>
@@ -324,7 +324,7 @@ int main(int argc, const char **argv) {<br>
if (DumpConfig) {<br>
std::string Config =<br>
clang::format::configurationAsText(clang::format::getStyle(<br>
- Style, FileNames.empty() ? AssumeFilename : FileNames[0],<br>
+ Style, FileNames.empty() ? AssumeFileName : FileNames[0],<br>
FallbackStyle));<br>
llvm::outs() << Config << "\n";<br>
return 0;<br>
<br>
Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=248782&r1=248781&r2=248782&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=248782&r1=248781&r2=248782&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Tue Sep 29 02:53:08 2015<br>
@@ -20,13 +20,15 @@ namespace {<br>
<br>
class SortIncludesTest : public ::testing::Test {<br>
protected:<br>
- std::string sort(llvm::StringRef Code) {<br>
+ std::string sort(llvm::StringRef Code, StringRef FileName = "input.cpp") {<br>
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));<br>
- std::string Sorted = applyAllReplacements(<br>
- Code, sortIncludes(getLLVMStyle(), Code, Ranges, "input.cpp"));<br>
- return applyAllReplacements(<br>
- Sorted, reformat(getLLVMStyle(), Sorted, Ranges, "input.cpp"));<br>
+ std::string Sorted =<br>
+ applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));<br>
+ return applyAllReplacements(Sorted,<br>
+ reformat(Style, Sorted, Ranges, FileName));<br>
}<br>
+<br>
+ FormatStyle Style = getLLVMStyle();<br>
};<br>
<br>
TEST_F(SortIncludesTest, BasicSorting) {<br>
@@ -76,13 +78,23 @@ TEST_F(SortIncludesTest, SortsLocallyInE<br>
"#include \"c.h\"\n"<br>
"\n"<br>
"#include \"b.h\"\n",<br>
- sort("#include \"c.h\"\n"<br>
- "#include \"a.h\"\n"<br>
+ sort("#include \"a.h\"\n"<br>
+ "#include \"c.h\"\n"<br>
"\n"<br>
"#include \"b.h\"\n"));<br>
}<br>
<br>
TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {<br>
+ EXPECT_EQ("#include \"a.h\"\n"<br>
+ "#include \"c.h\"\n"<br>
+ "#include <b.h>\n"<br>
+ "#include <d.h>\n",<br>
+ sort("#include <d.h>\n"<br>
+ "#include <b.h>\n"<br>
+ "#include \"c.h\"\n"<br>
+ "#include \"a.h\"\n"));<br>
+<br>
+ Style = getGoogleStyle(FormatStyle::LK_Cpp);<br>
EXPECT_EQ("#include <b.h>\n"<br>
"#include <d.h>\n"<br>
"#include \"a.h\"\n"<br>
@@ -103,6 +115,24 @@ TEST_F(SortIncludesTest, HandlesMultilin<br>
"#include \"b.h\"\n"));<br>
}<br>
<br>
+TEST_F(SortIncludesTest, LeavesMainHeaderFirst) {<br>
+ EXPECT_EQ("#include \"llvm/a.h\"\n"<br>
+ "#include \"b.h\"\n"<br>
+ "#include \"c.h\"\n",<br>
+ sort("#include \"llvm/a.h\"\n"<br>
+ "#include \"c.h\"\n"<br>
+ "#include \"b.h\"\n"));<br>
+<br>
+ // Don't do this in headers.<br>
+ EXPECT_EQ("#include \"b.h\"\n"<br>
+ "#include \"c.h\"\n"<br>
+ "#include \"llvm/a.h\"\n",<br>
+ sort("#include \"llvm/a.h\"\n"<br>
+ "#include \"c.h\"\n"<br>
+ "#include \"b.h\"\n",<br>
+ "some_header.h"));<br>
+}<br>
+<br>
} // end namespace<br>
} // end namespace format<br>
} // end namespace clang<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>