hokein created this revision.
hokein added a reviewer: bkramer.
hokein added subscribers: ioeric, cfe-commits.
Some changes in the patch:
* Add two commandline flags in clang-include-fixer.
* Introduce a IncludeFixerContext for the queried symbol.
* Pull out CreateReplacementsForHeader.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===================================================================
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique<include_fixer::InMemorySymbolIndex>(Symbols));
- std::set<std::string> Headers;
- std::vector<clang::tooling::Replacement> Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
- "llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(&Factory, Code, "input.cc", ExtraArgs);
+ std::vector<clang::tooling::Replacement> Replacements;
+ if (!FixerContext.Headers.empty()) {
+ Replacements = clang::include_fixer::CreateReplacementsForHeader(
+ Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+ FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: include-fixer/tool/clang-include-fixer.py
===================================================================
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -28,6 +28,22 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
+ choosing_mode = vim.eval('g:clang_include_fixer_choosing_mode')
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message, choices, default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,13 +57,34 @@
buf = vim.current.buffer
text = '\n'.join(buf)
+ if choosing_mode:
+ # Run command to get all headers.
+ command = [binary, "-output-headers", "-db="+args.db, "-input="+args.input,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) == 0:
+ return
+ symbol = lines[0]
+ choices_message = ""
+ index = 1;
+ for line in lines[1:]:
+ choices_message += "&" + str(index) + line + "\n"
+ index += 1
+ select = ShowDialog("choose a header file for {0}.".format(symbol), choices_message)
+ # Insert a selected header.
+ command = [binary, "-insert-header="+lines[select], vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ vim.current.buffer[:] = None;
+ vim.current.buffer.append(stdout.splitlines())
+ print "Added #include {0}.".format(lines[select])
+ return;
+
+
# Call clang-include-fixer.
command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
# If successful, replace buffer contents.
if stderr:
Index: include-fixer/tool/ClangIncludeFixer.cpp
===================================================================
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -9,13 +9,16 @@
#include "InMemorySymbolIndex.h"
#include "IncludeFixer.h"
+#include "IncludeFixerContext.h"
#include "SymbolIndexManager.h"
#include "YamlSymbolIndex.h"
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/ReplacementsYaml.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/YAMLTraits.h"
using namespace clang;
using namespace llvm;
@@ -56,6 +59,23 @@
"used for editor integration."),
cl::init(false), cl::cat(IncludeFixerCategory));
+cl::opt<bool> OuputHeaders(
+ "output-headers",
+ cl::desc("Output the queried symbol and its relevant headers.\n"
+ "The first line is the symbol name. The other lines\n"
+ "are the headers: \n"
+ " b::foo\n"
+ " path/to/foo_a.h\n"
+ " path/to/foo_b.h\n"),
+ cl::init(false), cl::cat(IncludeFixerCategory));
+
+cl::opt<std::string>
+ InsertHeader("insert-header",
+ cl::desc("Insert a specific header to a file.\n"
+ " clang-include-fixer -insert-header=\"foo.h\""
+ " path/to/source.cc"),
+ cl::init(""), cl::cat(IncludeFixerCategory));
+
cl::opt<std::string>
Style("style",
cl::desc("Fallback style for reformatting after inserting new "
@@ -87,6 +107,29 @@
tool.mapVirtualFile(options.getSourcePathList().front(), Code->getBuffer());
}
+ if (!InsertHeader.empty()) {
+ StringRef FilePath = options.getSourcePathList().front();
+ auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
+ if (!Buffer) {
+ errs() << "Couldn't open file: " << FilePath;
+ return 1;
+ }
+
+ // FIXME: Insert the header in the FirstIncludeOffset.
+ std::vector<tooling::Replacement> Replacements =
+ clang::include_fixer::CreateReplacementsForHeader(
+ /*Code=*/Buffer.get()->getBuffer(),
+ /*FilePath=*/FilePath,
+ /*FallbackStyle=*/Style,
+ /*FirstIncludeOffset=*/-1U,
+ /*Header=*/InsertHeader);
+ tooling::Replacements Replaces(Replacements.begin(), Replacements.end());
+ std::string ChangedCode =
+ tooling::applyAllReplacements(Buffer.get()->getBuffer(), Replaces);
+ llvm::outs() << ChangedCode;
+ return 0;
+ }
+
// Set up data source.
auto SymbolIndexMgr = llvm::make_unique<include_fixer::SymbolIndexManager>();
switch (DatabaseFormat) {
@@ -139,20 +182,45 @@
}
// Now run our tool.
- std::set<std::string> Headers; // Headers to be added.
- std::vector<tooling::Replacement> Replacements;
- include_fixer::IncludeFixerActionFactory Factory(
- *SymbolIndexMgr, Headers, Replacements, Style, MinimizeIncludePaths);
+ include_fixer::IncludeFixerContext Context;
+ include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Context,
+ Style, MinimizeIncludePaths);
if (tool.run(&Factory) != 0) {
llvm::errs()
<< "Clang died with a fatal error! (incorrect include paths?)\n";
return 1;
}
+ if (Context.Headers.empty())
+ return 0;
+
+ StringRef FilePath = options.getSourcePathList().front();
+ auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
+ if (!Buffer) {
+ errs() << "Couldn't open file: " << FilePath;
+ return 1;
+ }
+
+ // FIXME: Rank the results and pick the best one instead of the first one.
+ std::vector<tooling::Replacement> Replacements =
+ clang::include_fixer::CreateReplacementsForHeader(
+ /*Code=*/Buffer.get()->getBuffer(),
+ /*FilePath=*/FilePath,
+ /*FallbackStyle=*/Style,
+ /*FirstIncludeOffset=*/Context.FirstIncludeOffset,
+ /*Header=*/Context.Headers.front());
+
if (!Quiet)
- for (const auto &Header : Headers)
- llvm::errs() << "Added #include " << Header;
+ llvm::errs() << "Added #include" << Context.Headers.front();
+
+ if (OuputHeaders) {
+ // FIXME: Output all the fields in Context.
+ llvm::outs() << Context.SymbolIdentifer << "\n";
+ for (const auto &Header : Context.Headers)
+ llvm::outs() << Header << "\n";
+ return 0;
+ }
// Set up a new source manager for applying the resulting replacements.
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
Index: include-fixer/IncludeFixerContext.h
===================================================================
--- /dev/null
+++ include-fixer/IncludeFixerContext.h
@@ -0,0 +1,28 @@
+//===-- IncludeFixerContext.h - Include fixer context -----------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXERCONTEXT_H
+#define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXERCONTEXT_H
+
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace include_fixer {
+
+struct IncludeFixerContext {
+ std::string SymbolIdentifer;
+ std::vector<std::string> Headers;
+ unsigned FirstIncludeOffset;
+};
+
+} // namespace include_fixer
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXERCONTEXT_H
Index: include-fixer/IncludeFixer.h
===================================================================
--- include-fixer/IncludeFixer.h
+++ include-fixer/IncludeFixer.h
@@ -10,6 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H
#define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H
+#include "IncludeFixerContext.h"
#include "SymbolIndexManager.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Tooling.h"
@@ -31,10 +32,9 @@
/// \param Replacements Storage for the output of the fixer.
/// \param StyleName Fallback style for reformatting.
/// \param MinimizeIncludePaths whether inserted include paths are optimized.
- IncludeFixerActionFactory(
- SymbolIndexManager &SymbolIndexMgr, std::set<std::string> &Headers,
- std::vector<clang::tooling::Replacement> &Replacements,
- StringRef StyleName, bool MinimizeIncludePaths = true);
+ IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,
+ IncludeFixerContext &Headers, StringRef StyleName,
+ bool MinimizeIncludePaths = true);
~IncludeFixerActionFactory() override;
@@ -48,11 +48,8 @@
/// The client to use to find cross-references.
SymbolIndexManager &SymbolIndexMgr;
- /// Headers to be added.
- std::set<std::string> &Headers;
-
- /// Replacements are written here.
- std::vector<clang::tooling::Replacement> &Replacements;
+ /// The context that contains all information about queried symbol.
+ IncludeFixerContext &Context;
/// Whether inserted include paths should be optimized.
bool MinimizeIncludePaths;
@@ -62,6 +59,18 @@
std::string FallbackStyle;
};
+/// Insert a header before the first #include in \p Code and run
+/// clang-format to sort all headers.
+/// \param Code The source code.
+/// \param FilePath The source file path.
+/// \param StyleName Fallback style for reformatting.
+/// \param FirstIncludeOffset The insertion point for new include directives.
+/// \param Header The header being inserted.
+/// \return Replacements for inserting and sorting headers.
+std::vector<clang::tooling::Replacement> CreateReplacementsForHeader(
+ StringRef Code, StringRef FilePath, StringRef FallbackStyle,
+ unsigned FirstIncludeOffset, const std::string &Header);
+
} // namespace include_fixer
} // namespace clang
Index: include-fixer/IncludeFixer.cpp
===================================================================
--- include-fixer/IncludeFixer.cpp
+++ include-fixer/IncludeFixer.cpp
@@ -236,110 +236,48 @@
return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
}
- /// Insert all headers before the first #include in \p Code and run
- /// clang-format to sort all headers.
- /// \return Replacements for inserting and sorting headers.
- std::vector<clang::tooling::Replacement>
- CreateReplacementsForHeaders(StringRef Code,
- const std::set<std::string> &Headers) {
- // Create replacements for new headers.
- clang::tooling::Replacements Insertions;
- if (FirstIncludeOffset == -1U) {
- // FIXME: skip header guards.
- FirstIncludeOffset = 0;
- // If there is no existing #include, then insert an empty line after new
- // header block.
- if (Code.front() != '\n')
- Insertions.insert(
- clang::tooling::Replacement(Filename, FirstIncludeOffset, 0, "\n"));
- }
- // Keep inserting new headers before the first header.
- for (StringRef Header : Headers) {
- std::string Text = "#include " + Header.str() + "\n";
- Insertions.insert(
- clang::tooling::Replacement(Filename, FirstIncludeOffset, 0, Text));
- }
- DEBUG({
- llvm::dbgs() << "Header insertions:\n";
- for (const auto &R : Insertions)
- llvm::dbgs() << R.toString() << '\n';
- });
-
- clang::format::FormatStyle Style =
- clang::format::getStyle("file", Filename, FallbackStyle);
- clang::tooling::Replacements Replaces =
- formatReplacements(Code, Insertions, Style);
- // FIXME: remove this when `clang::tooling::Replacements` is implemented as
- // `std::vector<clang::tooling::Replacement>`.
- std::vector<clang::tooling::Replacement> Results;
- std::copy(Replaces.begin(), Replaces.end(), std::back_inserter(Results));
- return Results;
- }
-
/// Generate replacements for the suggested includes.
/// \return true if changes will be made, false otherwise.
- bool Rewrite(clang::SourceManager &SourceManager,
- clang::HeaderSearch &HeaderSearch,
- std::set<std::string> &Headers,
- std::vector<clang::tooling::Replacement> &Replacements) {
- if (Untried.empty())
+ bool MinimizeAllIncludeHeaders(clang::SourceManager &SourceManager,
+ clang::HeaderSearch &HeaderSearch,
+ IncludeFixerContext &Context) {
+ if (SymbolQueryResults.empty())
return false;
- const auto &ToTry = UntriedList.front();
- Headers.insert(minimizeInclude(ToTry, SourceManager, HeaderSearch));
+ Context.SymbolIdentifer = QuerySymbol;
+ Context.FirstIncludeOffset = FirstIncludeOffset;
+ for (const auto &Header : SymbolQueryResults)
+ Context.Headers.push_back(
+ minimizeInclude(Header, SourceManager, HeaderSearch));
- StringRef Code = SourceManager.getBufferData(SourceManager.getMainFileID());
- Replacements = CreateReplacementsForHeaders(Code, Headers);
-
- // We currently abort after the first inserted include. The more
- // includes we have the less safe this becomes due to error recovery
- // changing the results.
- // FIXME: Handle multiple includes at once.
return true;
}
/// Sets the location at the very top of the file.
void setFileBegin(clang::SourceLocation Location) { FileBegin = Location; }
- /// Add an include to the set of includes to try.
- /// \param include_path The include path to try.
- void TryInclude(const std::string &query, const std::string &include_path) {
- if (Untried.insert(include_path).second)
- UntriedList.push_back(include_path);
- }
-
private:
/// Query the database for a given identifier.
bool query(StringRef Query, SourceLocation Loc) {
assert(!Query.empty() && "Empty query!");
- // Save database lookups by not looking up identifiers multiple times.
- if (!SeenQueries.insert(Query).second)
- return true;
+ // Skip other identifers once we have discovered an identfier successfully.
+ if (!SymbolQueryResults.empty())
+ return false;
DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
DEBUG(Loc.print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
DEBUG(llvm::dbgs() << " ...");
- std::string error_text;
- auto SearchReply = SymbolIndexMgr.search(Query);
- DEBUG(llvm::dbgs() << SearchReply.size() << " replies\n");
- if (SearchReply.empty())
- return false;
-
- // Add those files to the set of includes to try out.
- // FIXME: Rank the results and pick the best one instead of the first one.
- TryInclude(Query, SearchReply[0]);
-
- return true;
+ QuerySymbol = Query.str();
+ SymbolQueryResults = SymbolIndexMgr.search(Query);
+ DEBUG(llvm::dbgs() << SymbolQueryResults.size() << " replies\n");
+ return !SymbolQueryResults.empty();
}
/// The client to use to find cross-references.
SymbolIndexManager &SymbolIndexMgr;
- // Remeber things we looked up to avoid querying things twice.
- llvm::StringSet<> SeenQueries;
-
/// The absolute path to the file being processed.
std::string Filename;
@@ -354,11 +292,12 @@
/// clang-format config file found.
std::string FallbackStyle;
- /// Includes we have left to try. A set to unique them and a list to keep
- /// track of the order. We prefer includes that were discovered early to avoid
- /// getting caught in results from error recovery.
- std::set<std::string> Untried;
- std::vector<std::string> UntriedList;
+ /// The symbol being queried.
+ std::string QuerySymbol;
+
+ /// The query results of an identifier. We only include the first discovered
+ /// identifier to avoid getting caught in results from error recovery.
+ std::vector<std::string> SymbolQueryResults;
/// Whether we should use the smallest possible include path.
bool MinimizeIncludePaths = true;
@@ -405,12 +344,10 @@
} // namespace
IncludeFixerActionFactory::IncludeFixerActionFactory(
- SymbolIndexManager &SymbolIndexMgr, std::set<std::string> &Headers,
- std::vector<clang::tooling::Replacement> &Replacements, StringRef StyleName,
- bool MinimizeIncludePaths)
- : SymbolIndexMgr(SymbolIndexMgr), Headers(Headers),
- Replacements(Replacements), MinimizeIncludePaths(MinimizeIncludePaths),
- FallbackStyle(StyleName) {}
+ SymbolIndexManager &SymbolIndexMgr, IncludeFixerContext &Context,
+ StringRef StyleName, bool MinimizeIncludePaths)
+ : SymbolIndexMgr(SymbolIndexMgr), Context(Context),
+ MinimizeIncludePaths(MinimizeIncludePaths), FallbackStyle(StyleName) {}
IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
@@ -441,15 +378,52 @@
Compiler.ExecuteAction(*ScopedToolAction);
// Generate replacements.
- ScopedToolAction->Rewrite(Compiler.getSourceManager(),
- Compiler.getPreprocessor().getHeaderSearchInfo(),
- Headers, Replacements);
+ ScopedToolAction->MinimizeAllIncludeHeaders(
+ Compiler.getSourceManager(),
+ Compiler.getPreprocessor().getHeaderSearchInfo(), Context);
// Technically this should only return true if we're sure that we have a
// parseable file. We don't know that though. Only inform users of fatal
// errors.
return !Compiler.getDiagnostics().hasFatalErrorOccurred();
}
+std::vector<clang::tooling::Replacement> CreateReplacementsForHeader(
+ StringRef Code, StringRef FilePath, StringRef FallbackStyle,
+ unsigned FirstIncludeOffset, const std::string &Header) {
+ if (Header.empty())
+ return {};
+ // Create replacements for new headers.
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+ // FIXME: skip header guards.
+ FirstIncludeOffset = 0;
+ // If there is no existing #include, then insert an empty line after new
+ // header block.
+ if (Code.front() != '\n')
+ Insertions.insert(
+ clang::tooling::Replacement(FilePath, FirstIncludeOffset, 0, "\n"));
+ }
+ // Keep inserting new headers before the first header.
+ std::string Text = "#include " + Header + "\n";
+ Insertions.insert(
+ clang::tooling::Replacement(FilePath, FirstIncludeOffset, 0, Text));
+ DEBUG({
+ llvm::dbgs() << "Header insertions:\n";
+ for (const auto &R : Insertions)
+ llvm::dbgs() << R.toString() << '\n';
+ });
+
+ clang::format::FormatStyle Style =
+ clang::format::getStyle("file", FilePath, FallbackStyle);
+ clang::tooling::Replacements Replaces =
+ formatReplacements(Code, Insertions, Style);
+ // FIXME: remove this when `clang::tooling::Replacements` is implemented as
+ // `std::vector<clang::tooling::Replacement>`.
+ std::vector<clang::tooling::Replacement> Results;
+ std::copy(Replaces.begin(), Replaces.end(), std::back_inserter(Results));
+ return Results;
+}
+
} // namespace include_fixer
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
