hokein updated this revision to Diff 67171.
hokein added a comment.
Remove unneeded header.
https://reviews.llvm.org/D23266
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===================================================================
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -88,15 +88,15 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique<include_fixer::InMemorySymbolIndex>(Symbols));
- IncludeFixerContext FixerContext;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+ std::vector<IncludeFixerContext> FixerContexts;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm");
std::string FakeFileName = "input.cc";
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
- if (FixerContext.getHeaderInfos().empty())
+ assert(FixerContexts.size() == 1);
+ if (FixerContexts.front().getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
- Code, FakeFileName, FixerContext);
+ Code, FixerContexts.front());
EXPECT_TRUE(static_cast<bool>(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
Index: test/include-fixer/commandline_options.cpp
===================================================================
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
-// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
+// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",
Index: include-fixer/tool/clang-include-fixer.py
===================================================================
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -149,21 +149,16 @@
return
try:
- # If there is only one suggested header, insert it directly.
- if len(unique_headers) == 1 or maximum_suggested_headers == 1:
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": header_infos}, text)
- print "Added #include {0} for {1}.".format(unique_headers[0], symbol)
- return
-
- selected = GetUserSelection("choose a header file for {0}.".format(symbol),
- unique_headers, maximum_suggested_headers)
- selected_header_infos = [
+ inserted_header_infos = header_infos
+ if len(unique_headers) > 1:
+ selected = GetUserSelection(
+ "choose a header file for {0}.".format(symbol),
+ unique_headers, maximum_suggested_headers)
+ inserted_header_infos = [
header for header in header_infos if header["Header"] == selected]
+ include_fixer_context["HeaderInfos"] = inserted_header_infos
- # Insert a selected header.
- InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
- "HeaderInfos": selected_header_infos}, text)
+ InsertHeaderToVimBuffer(include_fixer_context, text)
print "Added #include {0} for {1}.".format(selected, symbol)
except Exception as error:
print >> sys.stderr, error.message
Index: include-fixer/tool/ClangIncludeFixer.cpp
===================================================================
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -73,6 +73,7 @@
static void mapping(IO &IO, IncludeFixerContext &Context) {
IO.mapRequired("QuerySymbolInfos", Context.QuerySymbolInfos);
IO.mapRequired("HeaderInfos", Context.HeaderInfos);
+ IO.mapRequired("FilePath", Context.FilePath);
}
};
} // namespace yaml
@@ -203,7 +204,9 @@
void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) {
OS << "{\n"
- " \"QuerySymbolInfos\": [\n";
+ << " \"FilePath\": \""
+ << llvm::yaml::escape(Context.getFilePath()) << "\",\n"
+ << " \"QuerySymbolInfos\": [\n";
for (const auto &Info : Context.getQuerySymbolInfos()) {
OS << " {\"RawIdentifier\": \"" << Info.RawIdentifier << "\",\n";
OS << " \"Range\":{";
@@ -251,9 +254,6 @@
tool.mapVirtualFile(options.getSourcePathList().front(), Code->getBuffer());
}
- StringRef FilePath = options.getSourcePathList().front();
- format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style);
-
if (!InsertHeader.empty()) {
if (!STDINMode) {
errs() << "Should be running in STDIN mode\n";
@@ -289,9 +289,10 @@
const IncludeFixerContext::HeaderInfo &RHS) {
return LHS.QualifiedName == RHS.QualifiedName;
});
-
+ format::FormatStyle InsertStyle =
+ format::getStyle("file", Context.getFilePath(), Style);
auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
- Code->getBuffer(), FilePath, Context, InsertStyle,
+ Code->getBuffer(), Context, InsertStyle,
/*AddQualifiers=*/IsUniqueQualifiedName);
if (!Replacements) {
errs() << "Failed to create replacements: "
@@ -316,64 +317,83 @@
return 1;
// Now run our tool.
- include_fixer::IncludeFixerContext Context;
- include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Context,
+ std::vector<include_fixer::IncludeFixerContext> Contexts;
+ include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Contexts,
Style, MinimizeIncludePaths);
if (tool.run(&Factory) != 0) {
llvm::errs()
<< "Clang died with a fatal error! (incorrect include paths?)\n";
return 1;
}
+ assert(!Contexts.empty());
+
if (OutputHeaders) {
- writeToJson(llvm::outs(), Context);
+ // FIXME: Print contexts of all processing files instead of the first one.
+ writeToJson(llvm::outs(), Contexts.front());
return 0;
}
- if (Context.getHeaderInfos().empty())
- return 0;
+ std::vector<tooling::Replacements> FixerReplacements;
+ for (const auto &Context : Contexts) {
+ StringRef FilePath = Context.getFilePath();
+ format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style);
+ auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
+ if (!Buffer) {
+ errs() << "Couldn't open file: " + FilePath.str() + ": "
+ << Buffer.getError().message() + "\n";
+ return 1;
+ }
- auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
- if (!Buffer) {
- errs() << "Couldn't open file: " << FilePath << ": "
- << Buffer.getError().message() << '\n';
- return 1;
+ auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
+ Buffer.get()->getBuffer(), Context, InsertStyle);
+ if (!Replacements) {
+ errs() << "Failed to create replacement: "
+ << llvm::toString(Replacements.takeError()) << "\n";
+ return 1;
+ }
+ FixerReplacements.push_back(*Replacements);
}
- auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
- /*Code=*/Buffer.get()->getBuffer(), FilePath, Context, InsertStyle);
- if (!Replacements) {
- errs() << "Failed to create header insertion replacement: "
- << llvm::toString(Replacements.takeError()) << "\n";
- return 1;
+ if (!Quiet) {
+ for (const auto &Context : Contexts) {
+ if (!Context.getHeaderInfos().empty()) {
+ llvm::errs() << "Added #include "
+ << Context.getHeaderInfos().front().Header << " for "
+ << Context.getFilePath() << "\n";
+ }
+ }
}
- if (!Quiet)
- llvm::errs() << "Added #include " << Context.getHeaderInfos().front().Header
- << "\n";
-
- // Set up a new source manager for applying the resulting replacements.
- IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
- DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts);
- TextDiagnosticPrinter DiagnosticPrinter(outs(), &*DiagOpts);
- SourceManager SM(Diagnostics, tool.getFiles());
- Diagnostics.setClient(&DiagnosticPrinter, false);
-
if (STDINMode) {
- auto ChangedCode =
- tooling::applyAllReplacements(Code->getBuffer(), *Replacements);
+ assert(FixerReplacements.size() == 1);
+ auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(),
+ FixerReplacements.front());
if (!ChangedCode) {
llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
return 1;
}
llvm::outs() << *ChangedCode;
return 0;
}
+ // Set up a new source manager for applying the resulting replacements.
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
+ DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts);
+ TextDiagnosticPrinter DiagnosticPrinter(outs(), &*DiagOpts);
+ SourceManager SM(Diagnostics, tool.getFiles());
+ Diagnostics.setClient(&DiagnosticPrinter, false);
+
// Write replacements to disk.
Rewriter Rewrites(SM, LangOptions());
- tooling::applyAllReplacements(*Replacements, Rewrites);
+ for (const auto Replacement : FixerReplacements) {
+ if (!tooling::applyAllReplacements(Replacement, Rewrites)) {
+ llvm::errs() << "Failed to apply replacements for "
+ << Replacement.begin()->getFilePath() << '\n';
+ return 1;
+ }
+ }
return Rewrites.overwriteChangedFiles();
}
Index: include-fixer/IncludeFixerContext.h
===================================================================
--- include-fixer/IncludeFixerContext.h
+++ include-fixer/IncludeFixerContext.h
@@ -18,7 +18,8 @@
namespace clang {
namespace include_fixer {
-/// \brief A context for the symbol being queried.
+/// \brief A context for the file being processed. It includes all query
+/// information, e.g. symbols being queried in database, all header candiates.
class IncludeFixerContext {
public:
struct HeaderInfo {
@@ -46,7 +47,8 @@
};
IncludeFixerContext() = default;
- IncludeFixerContext(std::vector<QuerySymbolInfo> QuerySymbols,
+ IncludeFixerContext(StringRef FilePath,
+ std::vector<QuerySymbolInfo> QuerySymbols,
std::vector<find_all_symbols::SymbolInfo> Symbols);
/// \brief Get symbol name.
@@ -59,6 +61,9 @@
return QuerySymbolInfos.front().Range;
}
+ /// \brief Get the absolute file path.
+ StringRef getFilePath() const { return FilePath; }
+
/// \brief Get header information.
const std::vector<HeaderInfo> &getHeaderInfos() const { return HeaderInfos; }
@@ -70,6 +75,9 @@
private:
friend struct llvm::yaml::MappingTraits<IncludeFixerContext>;
+ /// \brief The absolute path to the file being processed.
+ std::string FilePath;
+
/// \brief All instances of an unidentified symbol being queried.
std::vector<QuerySymbolInfo> QuerySymbolInfos;
Index: include-fixer/IncludeFixerContext.cpp
===================================================================
--- include-fixer/IncludeFixerContext.cpp
+++ include-fixer/IncludeFixerContext.cpp
@@ -76,9 +76,9 @@
} // anonymous namespace
IncludeFixerContext::IncludeFixerContext(
- std::vector<QuerySymbolInfo> QuerySymbols,
+ StringRef FilePath, std::vector<QuerySymbolInfo> QuerySymbols,
std::vector<find_all_symbols::SymbolInfo> Symbols)
- : QuerySymbolInfos(std::move(QuerySymbols)),
+ : FilePath(FilePath), QuerySymbolInfos(std::move(QuerySymbols)),
MatchedSymbols(std::move(Symbols)) {
// Remove replicated QuerySymbolInfos with the same range.
//
Index: include-fixer/IncludeFixer.h
===================================================================
--- include-fixer/IncludeFixer.h
+++ include-fixer/IncludeFixer.h
@@ -34,7 +34,8 @@
/// \param StyleName Fallback style for reformatting.
/// \param MinimizeIncludePaths whether inserted include paths are optimized.
IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,
- IncludeFixerContext &Context, StringRef StyleName,
+ std::vector<IncludeFixerContext> &Contexts,
+ StringRef StyleName,
bool MinimizeIncludePaths = true);
~IncludeFixerActionFactory() override;
@@ -50,7 +51,7 @@
SymbolIndexManager &SymbolIndexMgr;
/// The context that contains all information about the symbol being queried.
- IncludeFixerContext &Context;
+ std::vector<IncludeFixerContext> &Contexts;
/// Whether inserted include paths should be optimized.
bool MinimizeIncludePaths;
@@ -65,7 +66,6 @@
/// first header for insertion.
///
/// \param Code The source code.
-/// \param FilePath The source file path.
/// \param Context The context which contains all information for creating
/// include-fixer replacements.
/// \param Style clang-format style being used.
@@ -76,7 +76,7 @@
/// qualifiers on success; otherwise, an llvm::Error carrying llvm::StringError
/// is returned.
llvm::Expected<tooling::Replacements> createIncludeFixerReplacements(
- StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+ StringRef Code, const IncludeFixerContext &Context,
const format::FormatStyle &Style = format::getLLVMStyle(),
bool AddQualifiers = true);
Index: include-fixer/IncludeFixer.cpp
===================================================================
--- include-fixer/IncludeFixer.cpp
+++ include-fixer/IncludeFixer.cpp
@@ -37,6 +37,7 @@
std::unique_ptr<clang::ASTConsumer>
CreateASTConsumer(clang::CompilerInstance &Compiler,
StringRef InFile) override {
+ FilePath = InFile;
return llvm::make_unique<clang::ASTConsumer>();
}
@@ -228,7 +229,7 @@
Symbol.getContexts(),
Symbol.getNumOccurrences());
}
- return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates);
+ return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates);
}
private:
@@ -297,16 +298,20 @@
/// recovery.
std::vector<find_all_symbols::SymbolInfo> MatchedSymbols;
+ /// The absolute path to the file being processed.
+ std::string FilePath;
+
/// Whether we should use the smallest possible include path.
bool MinimizeIncludePaths = true;
};
} // namespace
IncludeFixerActionFactory::IncludeFixerActionFactory(
- SymbolIndexManager &SymbolIndexMgr, IncludeFixerContext &Context,
- StringRef StyleName, bool MinimizeIncludePaths)
- : SymbolIndexMgr(SymbolIndexMgr), Context(Context),
+ SymbolIndexManager &SymbolIndexMgr,
+ std::vector<IncludeFixerContext> &Contexts, StringRef StyleName,
+ bool MinimizeIncludePaths)
+ : SymbolIndexMgr(SymbolIndexMgr), Contexts(Contexts),
MinimizeIncludePaths(MinimizeIncludePaths) {}
IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
@@ -337,21 +342,22 @@
llvm::make_unique<Action>(SymbolIndexMgr, MinimizeIncludePaths);
Compiler.ExecuteAction(*ScopedToolAction);
- Context = ScopedToolAction->getIncludeFixerContext(
+ Contexts.push_back(ScopedToolAction->getIncludeFixerContext(
Compiler.getSourceManager(),
- Compiler.getPreprocessor().getHeaderSearchInfo());
+ Compiler.getPreprocessor().getHeaderSearchInfo()));
// 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();
}
llvm::Expected<tooling::Replacements> createIncludeFixerReplacements(
- StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+ StringRef Code, const IncludeFixerContext &Context,
const clang::format::FormatStyle &Style, bool AddQualifiers) {
if (Context.getHeaderInfos().empty())
return tooling::Replacements();
+ StringRef FilePath = Context.getFilePath();
std::string IncludeName =
"#include " + Context.getHeaderInfos().front().Header + "\n";
// Create replacements for the new header.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
