This revision was automatically updated to reflect the committed changes.
Closed by commit rL271287: [include-fixer] use clang-format cleaner to insert
header. (authored by ioeric).
Changed prior to commit:
http://reviews.llvm.org/D20816?vs=59067&id=59069#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20816
Files:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -78,21 +78,22 @@
return Code;
tooling::Replacements Replacements =
clang::include_fixer::createInsertHeaderReplacements(
- Code, "input.cc", FixerContext.Headers.front(),
- FixerContext.FirstIncludeOffset);
+ Code, "input.cc", FixerContext.Headers.front());
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
return Context.getRewrittenText(ID);
}
TEST(IncludeFixer, Typo) {
- EXPECT_EQ("#include <string>\n\nstd::string foo;\n",
+ EXPECT_EQ("#include <string>\nstd::string foo;\n",
runIncludeFixer("std::string foo;\n"));
+ // FIXME: the current version of include-fixer does not get this test case
+ // right - header should be inserted before definition.
EXPECT_EQ(
- "// comment\n#include \"foo.h\"\n#include <string>\nstd::string foo;\n"
- "#include \"dir/bar.h\"\n",
+ "// comment\n#include \"foo.h\"\nstd::string foo;\n"
+ "#include \"dir/bar.h\"\n#include <string>\n",
runIncludeFixer("// comment\n#include \"foo.h\"\nstd::string foo;\n"
"#include \"dir/bar.h\"\n"));
@@ -106,11 +107,11 @@
// string without "std::" can also be fixed since fixed db results go through
// SymbolIndexManager, and SymbolIndexManager matches unqualified identifiers
// too.
- EXPECT_EQ("#include <string>\n\nstring foo;\n",
+ EXPECT_EQ("#include <string>\nstring foo;\n",
runIncludeFixer("string foo;\n"));
// Fully qualified name.
- EXPECT_EQ("#include <string>\n\n::std::string foo;\n",
+ EXPECT_EQ("#include <string>\n::std::string foo;\n",
runIncludeFixer("::std::string foo;\n"));
// Should not match std::string.
EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
@@ -126,24 +127,24 @@
TEST(IncludeFixer, MinimizeInclude) {
std::vector<std::string> IncludePath = {"-Idir/"};
- EXPECT_EQ("#include \"otherdir/qux.h\"\n\na::b::foo bar;\n",
+ EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
runIncludeFixer("a::b::foo bar;\n", IncludePath));
IncludePath = {"-isystemdir"};
- EXPECT_EQ("#include <otherdir/qux.h>\n\na::b::foo bar;\n",
+ EXPECT_EQ("#include <otherdir/qux.h>\na::b::foo bar;\n",
runIncludeFixer("a::b::foo bar;\n", IncludePath));
IncludePath = {"-iquotedir"};
- EXPECT_EQ("#include \"otherdir/qux.h\"\n\na::b::foo bar;\n",
+ EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
runIncludeFixer("a::b::foo bar;\n", IncludePath));
IncludePath = {"-Idir", "-Idir/otherdir"};
- EXPECT_EQ("#include \"qux.h\"\n\na::b::foo bar;\n",
+ EXPECT_EQ("#include \"qux.h\"\na::b::foo bar;\n",
runIncludeFixer("a::b::foo bar;\n", IncludePath));
}
TEST(IncludeFixer, NestedName) {
- EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n"
+ EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n"
"int x = a::b::foo(0);\n",
runIncludeFixer("int x = a::b::foo(0);\n"));
@@ -153,33 +154,35 @@
EXPECT_EQ("#define FOO(x) a::##x\nint x = FOO(b::foo);\n",
runIncludeFixer("#define FOO(x) a::##x\nint x = FOO(b::foo);\n"));
- EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n"
- "namespace a {}\nint a = a::b::foo(0);\n",
+ // The empty namespace is cleaned up by clang-format after include-fixer
+ // finishes.
+ EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n"
+ "\nint a = a::b::foo(0);\n",
runIncludeFixer("namespace a {}\nint a = a::b::foo(0);\n"));
}
TEST(IncludeFixer, MultipleMissingSymbols) {
- EXPECT_EQ("#include <string>\n\nstd::string bar;\nstd::sting foo;\n",
+ EXPECT_EQ("#include <string>\nstd::string bar;\nstd::sting foo;\n",
runIncludeFixer("std::string bar;\nstd::sting foo;\n"));
}
TEST(IncludeFixer, ScopedNamespaceSymbols) {
- EXPECT_EQ("#include \"bar.h\"\n\nnamespace a {\nb::bar b;\n}",
+ EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}",
runIncludeFixer("namespace a {\nb::bar b;\n}"));
- EXPECT_EQ("#include \"bar.h\"\n\nnamespace A {\na::b::bar b;\n}",
+ EXPECT_EQ("#include \"bar.h\"\nnamespace A {\na::b::bar b;\n}",
runIncludeFixer("namespace A {\na::b::bar b;\n}"));
- EXPECT_EQ("#include \"bar.h\"\n\nnamespace a {\nvoid func() { b::bar b; }\n}",
+ EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nvoid func() { b::bar b; }\n}",
runIncludeFixer("namespace a {\nvoid func() { b::bar b; }\n}"));
EXPECT_EQ("namespace A { c::b::bar b; }\n",
runIncludeFixer("namespace A { c::b::bar b; }\n"));
// FIXME: The header should not be added here. Remove this after we support
// full match.
- EXPECT_EQ("#include \"bar.h\"\n\nnamespace A {\nb::bar b;\n}",
+ EXPECT_EQ("#include \"bar.h\"\nnamespace A {\nb::bar b;\n}",
runIncludeFixer("namespace A {\nb::bar b;\n}"));
}
TEST(IncludeFixer, EnumConstantSymbols) {
- EXPECT_EQ("#include \"color.h\"\n\nint test = a::b::Green;\n",
+ EXPECT_EQ("#include \"color.h\"\nint test = a::b::Green;\n",
runIncludeFixer("int test = a::b::Green;\n"));
}
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
===================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h
@@ -60,23 +60,17 @@
std::string FallbackStyle;
};
-/// Create replacements for the header being inserted. The replacements will
-/// insert a header before the first #include in \p Code, and sort all headers
-/// with the given clang-format style.
+/// Create replacements, which are generated by clang-format, for the header
+/// insertion.
///
/// \param Code The source code.
/// \param FilePath The source file path.
/// \param Header The header being inserted.
-/// \param FirstIncludeOffset The insertion point for new include directives.
-/// The default value -1U means inserting the header at the first line, and if
-/// there is no #include block, it will create a header block by inserting a
-/// newline.
/// \param Style clang-format style being used.
///
/// \return Replacements for inserting and sorting headers.
tooling::Replacements createInsertHeaderReplacements(
StringRef Code, StringRef FilePath, StringRef Header,
- unsigned FirstIncludeOffset = -1U,
const clang::format::FormatStyle &Style = clang::format::getLLVMStyle());
} // namespace include_fixer
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@ -28,33 +28,6 @@
class Action;
-class PreprocessorHooks : public clang::PPCallbacks {
-public:
- explicit PreprocessorHooks(Action *EnclosingPass)
- : EnclosingPass(EnclosingPass), TrackedFile(nullptr) {}
-
- void FileChanged(clang::SourceLocation loc,
- clang::PPCallbacks::FileChangeReason reason,
- clang::SrcMgr::CharacteristicKind file_type,
- clang::FileID prev_fid) override;
-
- void InclusionDirective(clang::SourceLocation HashLocation,
- const clang::Token &IncludeToken,
- llvm::StringRef FileName, bool IsAngled,
- clang::CharSourceRange FileNameRange,
- const clang::FileEntry *IncludeFile,
- llvm::StringRef SearchPath,
- llvm::StringRef relative_path,
- const clang::Module *imported) override;
-
-private:
- /// The current Action.
- Action *EnclosingPass;
-
- /// The current FileEntry.
- const clang::FileEntry *TrackedFile;
-};
-
/// Manages the parse, gathers include suggestions.
class Action : public clang::ASTFrontendAction,
public clang::ExternalSemaSource {
@@ -67,8 +40,6 @@
CreateASTConsumer(clang::CompilerInstance &Compiler,
StringRef InFile) override {
Filename = InFile;
- Compiler.getPreprocessor().addPPCallbacks(
- llvm::make_unique<PreprocessorHooks>(this));
return llvm::make_unique<clang::ASTConsumer>();
}
@@ -195,22 +166,6 @@
StringRef filename() const { return Filename; }
- /// Called for each include file we discover is in the file.
- /// \param SourceManager the active SourceManager
- /// \param canonical_path the canonical path to the include file
- /// \param uttered_path the path as it appeared in the program
- /// \param IsAngled whether angle brackets were used
- /// \param HashLocation the source location of the include's \#
- /// \param EndLocation the source location following the include
- void NextInclude(clang::SourceManager *SourceManager,
- llvm::StringRef canonical_path, llvm::StringRef uttered_path,
- bool IsAngled, clang::SourceLocation HashLocation,
- clang::SourceLocation EndLocation) {
- unsigned Offset = SourceManager->getFileOffset(HashLocation);
- if (FirstIncludeOffset == -1U)
- FirstIncludeOffset = Offset;
- }
-
/// Get the minimal include for a given path.
std::string minimizeInclude(StringRef Include,
const clang::SourceManager &SourceManager,
@@ -244,17 +199,13 @@
return FixerContext;
FixerContext.SymbolIdentifer = QuerySymbol;
- FixerContext.FirstIncludeOffset = FirstIncludeOffset;
for (const auto &Header : SymbolQueryResults)
FixerContext.Headers.push_back(
minimizeInclude(Header, SourceManager, HeaderSearch));
return FixerContext;
}
- /// Sets the location at the very top of the file.
- void setFileBegin(clang::SourceLocation Location) { FileBegin = Location; }
-
private:
/// Query the database for a given identifier.
bool query(StringRef Query, SourceLocation Loc) {
@@ -280,13 +231,6 @@
/// The absolute path to the file being processed.
std::string Filename;
- /// The location of the beginning of the tracked file.
- clang::SourceLocation FileBegin;
-
- /// The offset of the last include in the original source file. This will
- /// be used as the insertion point for new include directives.
- unsigned FirstIncludeOffset = -1U;
-
/// The symbol being queried.
std::string QuerySymbol;
@@ -298,44 +242,6 @@
bool MinimizeIncludePaths = true;
};
-void PreprocessorHooks::FileChanged(clang::SourceLocation Loc,
- clang::PPCallbacks::FileChangeReason Reason,
- clang::SrcMgr::CharacteristicKind FileType,
- clang::FileID PrevFID) {
- // Remember where the main file starts.
- if (Reason == clang::PPCallbacks::EnterFile) {
- clang::SourceManager *SourceManager =
- &EnclosingPass->getCompilerInstance().getSourceManager();
- clang::FileID loc_id = SourceManager->getFileID(Loc);
- if (const clang::FileEntry *FileEntry =
- SourceManager->getFileEntryForID(loc_id)) {
- if (FileEntry->getName() == EnclosingPass->filename()) {
- EnclosingPass->setFileBegin(Loc);
- TrackedFile = FileEntry;
- }
- }
- }
-}
-
-void PreprocessorHooks::InclusionDirective(
- clang::SourceLocation HashLocation, const clang::Token &IncludeToken,
- llvm::StringRef FileName, bool IsAngled,
- clang::CharSourceRange FileNameRange, const clang::FileEntry *IncludeFile,
- llvm::StringRef SearchPath, llvm::StringRef relative_path,
- const clang::Module *imported) {
- // Remember include locations so we can insert our new include at the end of
- // the include block.
- clang::SourceManager *SourceManager =
- &EnclosingPass->getCompilerInstance().getSourceManager();
- auto IDPosition = SourceManager->getDecomposedExpansionLoc(HashLocation);
- const FileEntry *SourceFile =
- SourceManager->getFileEntryForID(IDPosition.first);
- if (!IncludeFile || TrackedFile != SourceFile)
- return;
- EnclosingPass->NextInclude(SourceManager, IncludeFile->getName(), FileName,
- IsAngled, HashLocation, FileNameRange.getEnd());
-}
-
} // namespace
IncludeFixerActionFactory::IncludeFixerActionFactory(
@@ -384,32 +290,17 @@
tooling::Replacements
createInsertHeaderReplacements(StringRef Code, StringRef FilePath,
- StringRef Header, unsigned FirstIncludeOffset,
+ StringRef Header,
const clang::format::FormatStyle &Style) {
if (Header.empty())
return tooling::Replacements();
- // 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.str() + "\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';
- });
+ std::string IncludeName = "#include " + Header.str() + "\n";
+ // Create replacements for the new header.
+ clang::tooling::Replacements Insertions = {
+ tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)};
- return formatReplacements(Code, Insertions, Style);
+ return formatReplacements(
+ Code, cleanupAroundReplacements(Code, Insertions, Style), Style);
}
} // namespace include_fixer
Index: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
@@ -168,11 +168,9 @@
return 1;
}
- // FIXME: Insert the header in right FirstIncludeOffset.
tooling::Replacements Replacements =
clang::include_fixer::createInsertHeaderReplacements(
- Code->getBuffer(), FilePath, InsertHeader,
- /*FirstIncludeOffset=*/0, InsertStyle);
+ Code->getBuffer(), FilePath, InsertHeader, InsertStyle);
tooling::Replacements Replaces(Replacements.begin(), Replacements.end());
std::string ChangedCode =
tooling::applyAllReplacements(Code->getBuffer(), Replaces);
@@ -218,7 +216,7 @@
tooling::Replacements Replacements =
clang::include_fixer::createInsertHeaderReplacements(
/*Code=*/Buffer.get()->getBuffer(), FilePath, Context.Headers.front(),
- Context.FirstIncludeOffset, InsertStyle);
+ InsertStyle);
if (!Quiet)
llvm::errs() << "Added #include" << Context.Headers.front();
Index: clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
===================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
+++ clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
@@ -22,8 +22,6 @@
std::string SymbolIdentifer;
/// \brief The headers which have SymbolIdentifier definitions.
std::vector<std::string> Headers;
- /// \brief The insertion point for new include header.
- unsigned FirstIncludeOffset;
};
} // namespace include_fixer
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
