djasper updated this revision to Diff 30176.
djasper marked 2 inline comments as done.
djasper added a comment.
Addressed review comments.
http://reviews.llvm.org/D11240
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/FormatToken.cpp
tools/clang-format/CMakeLists.txt
tools/clang-format/ClangFormat.cpp
tools/clang-format/clang-format.py
unittests/Format/CMakeLists.txt
unittests/Format/SortIncludesTest.cpp
Index: unittests/Format/SortIncludesTest.cpp
===================================================================
--- /dev/null
+++ unittests/Format/SortIncludesTest.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Format/SortIncludesTest.cpp - Include sort unit tests -----===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class SortIncludesTest : public ::testing::Test {
+protected:
+ std::string sort(llvm::StringRef Code) {
+ std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+ tooling::Replacements Replaces = sortIncludes(Code, Ranges, "input.cpp");
+ return applyAllReplacements(Code, Replaces);
+ }
+};
+
+TEST_F(SortIncludesTest, BasicSorting) {
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n",
+ sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, LeadingWhitespace) {
+ // Note that this will be fixed at a later step by the actualy formatting.
+ EXPECT_EQ(" #include \"a.h\"\n"
+ " #include \"b.h\"\n"
+ " #include \"c.h\"\n",
+ sort(" #include \"a.h\"\n"
+ " #include \"c.h\"\n"
+ " #include \"b.h\"\n"));
+ // Note that this will be fixed at a later step by the actualy formatting.
+ EXPECT_EQ("# include \"a.h\"\n"
+ "# include \"b.h\"\n"
+ "# include \"c.h\"\n",
+ sort("# include \"a.h\"\n"
+ "# include \"c.h\"\n"
+ "# include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, GreaterInComment) {
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"b.h\" // >\n"
+ "#include \"c.h\"\n",
+ sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\" // >\n"));
+}
+
+TEST_F(SortIncludesTest, SortsLocallyInEachBlock) {
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "#include \"b.h\"\n",
+ sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
+ EXPECT_EQ("#include <b.h>\n"
+ "#include <d.h>\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ sort("#include <d.h>\n"
+ "#include <b.h>\n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, HandlesMultilineIncludes) {
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \\\n"
+ "\"c.h\"\n",
+ sort("#include \"a.h\"\n"
+ "#include \\\n"
+ "\"c.h\"\n"
+ "#include \"b.h\"\n"));
+}
+
+} // end namespace
+} // end namespace format
+} // end namespace clang
Index: unittests/Format/CMakeLists.txt
===================================================================
--- unittests/Format/CMakeLists.txt
+++ unittests/Format/CMakeLists.txt
@@ -8,6 +8,7 @@
FormatTestJS.cpp
FormatTestProto.cpp
FormatTestSelective.cpp
+ SortIncludesTest.cpp
)
target_link_libraries(FormatTests
Index: tools/clang-format/clang-format.py
===================================================================
--- tools/clang-format/clang-format.py
+++ tools/clang-format/clang-format.py
@@ -72,7 +72,7 @@
startupinfo.wShowWindow = subprocess.SW_HIDE
# Call formatter.
- command = [binary, '-style', style, '-cursor', str(cursor)]
+ command = [binary, '-style', style, '-cursor', str(cursor), '-sort-includes']
if lines != 'all':
command.extend(['-lines', lines])
if fallback_style:
Index: tools/clang-format/ClangFormat.cpp
===================================================================
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -19,7 +19,6 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Version.h"
#include "clang/Format/Format.h"
-#include "clang/Rewrite/Core/Rewriter.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -97,6 +96,10 @@
"clang-format from an editor integration"),
cl::init(0), cl::cat(ClangFormatCategory));
+static cl::opt<bool> SortIncludes("sort-includes",
+ cl::desc("Sort touched include lines"),
+ cl::cat(ClangFormatCategory));
+
static cl::list<std::string> FileNames(cl::Positional, cl::desc("[<file> ...]"),
cl::cat(ClangFormatCategory));
@@ -121,9 +124,14 @@
LineRange.second.getAsInteger(0, ToLine);
}
-static bool fillRanges(SourceManager &Sources, FileID ID,
- const MemoryBuffer *Code,
- std::vector<CharSourceRange> &Ranges) {
+static bool fillRanges(MemoryBuffer *Code,
+ std::vector<tooling::Range> &Ranges) {
+ FileManager Files((FileSystemOptions()));
+ DiagnosticsEngine Diagnostics(
+ IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
+ new DiagnosticOptions);
+ SourceManager Sources(Diagnostics, Files);
+ FileID ID = createInMemoryFile("-", Code, Sources, Files);
if (!LineRanges.empty()) {
if (!Offsets.empty() || !Lengths.empty()) {
llvm::errs() << "error: cannot use -lines with -offset/-length\n";
@@ -144,7 +152,9 @@
SourceLocation End = Sources.translateLineCol(ID, ToLine, UINT_MAX);
if (Start.isInvalid() || End.isInvalid())
return true;
- Ranges.push_back(CharSourceRange::getCharRange(Start, End));
+ unsigned Offset = Sources.getFileOffset(Start);
+ unsigned Length = Sources.getFileOffset(End) - Offset;
+ Ranges.push_back(tooling::Range(Offset, Length));
}
return false;
}
@@ -177,7 +187,9 @@
} else {
End = Sources.getLocForEndOfFile(ID);
}
- Ranges.push_back(CharSourceRange::getCharRange(Start, End));
+ unsigned Offset = Sources.getFileOffset(Start);
+ unsigned Length = Sources.getFileOffset(End) - Offset;
+ Ranges.push_back(tooling::Range(Offset, Length));
}
return false;
}
@@ -202,13 +214,18 @@
llvm::outs() << Text.substr(From);
}
+static void outputReplacementsXML(const tooling::Replacements &Replaces) {
+ for (const auto &R : Replaces) {
+ outs() << "<replacement "
+ << "offset='" << R.getOffset() << "' "
+ << "length='" << R.getLength() << "'>";
+ outputReplacementXML(R.getReplacementText());
+ outs() << "</replacement>\n";
+ }
+}
+
// Returns true on error.
static bool format(StringRef FileName) {
- FileManager Files((FileSystemOptions()));
- DiagnosticsEngine Diagnostics(
- IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
- new DiagnosticOptions);
- SourceManager Sources(Diagnostics, Files);
ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
MemoryBuffer::getFileOrSTDIN(FileName);
if (std::error_code EC = CodeOrErr.getError()) {
@@ -218,16 +235,26 @@
std::unique_ptr<llvm::MemoryBuffer> Code = std::move(CodeOrErr.get());
if (Code->getBufferSize() == 0)
return false; // Empty files are formatted correctly.
- FileID ID = createInMemoryFile(FileName, Code.get(), Sources, Files);
- std::vector<CharSourceRange> Ranges;
- if (fillRanges(Sources, ID, Code.get(), Ranges))
+ std::vector<tooling::Range> Ranges;
+ if (fillRanges(Code.get(), Ranges))
return true;
+ tooling::Replacements IncludeReplacements;
+ std::string ChangedCode;
+ if (SortIncludes) {
+ StringRef CodeRef = Code.get()->getBuffer();
+ IncludeReplacements =
+ sortIncludes(Code.get()->getBuffer(), Ranges, FileName);
+ ChangedCode =
+ tooling::applyAllReplacements(CodeRef, IncludeReplacements);
+ } else {
+ ChangedCode = Code->getBuffer().str();
+ }
FormatStyle FormatStyle = getStyle(
Style, (FileName == "-") ? AssumeFilename : FileName, FallbackStyle);
bool IncompleteFormat = false;
- tooling::Replacements Replaces =
- reformat(FormatStyle, Sources, ID, Ranges, &IncompleteFormat);
+ tooling::Replacements Replaces = reformat(
+ FormatStyle, ChangedCode, Ranges, FileName, &IncompleteFormat);
if (OutputXML) {
llvm::outs() << "<?xml version='1.0'?>\n<replacements "
"xml:space='preserve' incomplete_format='"
@@ -237,31 +264,30 @@
<< tooling::shiftedCodePosition(Replaces, Cursor)
<< "</cursor>\n";
- for (tooling::Replacements::const_iterator I = Replaces.begin(),
- E = Replaces.end();
- I != E; ++I) {
- llvm::outs() << "<replacement "
- << "offset='" << I->getOffset() << "' "
- << "length='" << I->getLength() << "'>";
- outputReplacementXML(I->getReplacementText());
- llvm::outs() << "</replacement>\n";
- }
+ outputReplacementsXML(IncludeReplacements);
+ outputReplacementsXML(Replaces);
llvm::outs() << "</replacements>\n";
} else {
- Rewriter Rewrite(Sources, LangOptions());
- tooling::applyAllReplacements(Replaces, Rewrite);
+ std::string FormattedCode = applyAllReplacements(ChangedCode, Replaces);
if (Inplace) {
if (FileName == "-")
llvm::errs() << "error: cannot use -i when reading from stdin.\n";
- else if (Rewrite.overwriteChangedFiles())
- return true;
+ else {
+ std::error_code EC;
+ raw_fd_ostream FileOut(FileName, EC, llvm::sys::fs::F_Text);
+ if (EC) {
+ llvm::errs() << EC.message() << "\n";
+ return true;
+ }
+ FileOut << FormattedCode;
+ }
} else {
if (Cursor.getNumOccurrences() != 0)
outs() << "{ \"Cursor\": "
<< tooling::shiftedCodePosition(Replaces, Cursor)
<< ", \"IncompleteFormat\": "
<< (IncompleteFormat ? "true" : "false") << " }\n";
- Rewrite.getEditBuffer(ID).write(outs());
+ outs() << FormattedCode;
}
}
return false;
Index: tools/clang-format/CMakeLists.txt
===================================================================
--- tools/clang-format/CMakeLists.txt
+++ tools/clang-format/CMakeLists.txt
@@ -7,7 +7,6 @@
set(CLANG_FORMAT_LIB_DEPS
clangBasic
clangFormat
- clangRewrite
clangToolingCore
)
Index: lib/Format/FormatToken.cpp
===================================================================
--- lib/Format/FormatToken.cpp
+++ lib/Format/FormatToken.cpp
@@ -13,8 +13,8 @@
///
//===----------------------------------------------------------------------===//
-#include "FormatToken.h"
#include "ContinuationIndenter.h"
+#include "FormatToken.h"
#include "clang/Format/Format.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -309,8 +309,8 @@
return Seq[Index];
}
};
-}
-}
+} // namespace yaml
+} // namespace llvm
namespace clang {
namespace format {
@@ -1558,8 +1558,88 @@
bool BinPackInconclusiveFunctions;
};
+struct IncludeDirective {
+ StringRef Filename;
+ StringRef Text;
+ unsigned Offset;
+ bool IsAngled;
+};
+
} // end anonymous namespace
+// Determines whether 'Ranges' intersects with ('Start', 'End').
+static bool affectsRange(ArrayRef<tooling::Range> Ranges, unsigned Start,
+ unsigned End) {
+ for (auto Range : Ranges) {
+ if (Range.getOffset() < End &&
+ Range.getOffset() + Range.getLength() > Start)
+ return true;
+ }
+ return false;
+}
+
+// Sorts a block of includes given by 'Includes' alphabetically adding the
+// necessary replacement to 'Replaces'. 'Includes' must be in strict source
+// order.
+static void sortIncludes(const SmallVectorImpl<IncludeDirective> &Includes,
+ ArrayRef<tooling::Range> Ranges, StringRef FileName,
+ tooling::Replacements &Replaces) {
+ if (Includes.empty() ||
+ !affectsRange(Ranges, Includes.front().Offset,
+ Includes.back().Offset + Includes.back().Text.size()))
+ return;
+ SmallVector<unsigned, 16> Indices;
+ for (unsigned i = 0, e = Includes.size(); i != e; ++i)
+ Indices.push_back(i);
+ std::sort(Indices.begin(), Indices.end(), [&](unsigned LHSI, unsigned RHSI) {
+ return Includes[LHSI].Filename < Includes[RHSI].Filename;
+ });
+ for (unsigned i = 0, e = Indices.size(); i != e; ++i) {
+ if (i == Indices[i]) continue;
+ const IncludeDirective &To = Includes[i];
+ const IncludeDirective &From = Includes[Indices[i]];
+ Replaces.insert(
+ tooling::Replacement(FileName, To.Offset, To.Text.size(), From.Text));
+ }
+}
+
+tooling::Replacements sortIncludes(StringRef Code,
+ ArrayRef<tooling::Range> Ranges,
+ StringRef FileName) {
+ tooling::Replacements Replaces;
+ unsigned Prev = 0;
+ unsigned SearchFrom = 0;
+ llvm::Regex IncludeRegex(R"(^\ *#\ *include[^"<]*["<]([^">]*)([">]))");
+ SmallVector<StringRef, 4> Matches;
+ SmallVector<IncludeDirective, 16> IncludesInBlock;
+ for (;;) {
+ auto Pos = Code.find('\n', SearchFrom);
+ StringRef Line =
+ Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
+ if (!Line.endswith("\\")) {
+ if (IncludeRegex.match(Line, &Matches)) {
+ bool IsAngled = Matches[2] == ">";
+ if (!IncludesInBlock.empty() &&
+ IsAngled != IncludesInBlock.back().IsAngled) {
+ sortIncludes(IncludesInBlock, Ranges, FileName, Replaces);
+ IncludesInBlock.clear();
+ }
+ IncludesInBlock.push_back({Matches[1], Line, Prev, Matches[2] == ">"});
+ } else {
+ sortIncludes(IncludesInBlock, Ranges, FileName, Replaces);
+ IncludesInBlock.clear();
+ }
+ Prev = Pos + 1;
+ }
+ if (Pos == StringRef::npos || Pos + 1 == Code.size())
+ break;
+ SearchFrom = Pos + 1;
+ }
+ if (!IncludesInBlock.empty())
+ sortIncludes(IncludesInBlock, Ranges, FileName, Replaces);
+ return Replaces;
+}
+
tooling::Replacements reformat(const FormatStyle &Style,
SourceManager &SourceMgr, FileID ID,
ArrayRef<CharSourceRange> Ranges,
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -569,6 +569,12 @@
/// \brief Gets configuration in a YAML string.
std::string configurationAsText(const FormatStyle &Style);
+/// \brief Returns the replacements necessary to sort all #include blocks that
+/// are affected by 'Ranges'.
+tooling::Replacements sortIncludes(StringRef Code,
+ ArrayRef<tooling::Range> Ranges,
+ StringRef FileName);
+
/// \brief Reformats the given \p Ranges in the file \p ID.
///
/// Each range is extended on either end to its next bigger logic unit, i.e.
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
