Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions clang-tools-extra/include-cleaner/test/tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,13 @@ int x = foo();
// RUN: clang-include-cleaner -edit --ignore-headers="foobar\.h,foo\.h" %t.cpp -- -I%S/Inputs/
// RUN: FileCheck --match-full-lines --check-prefix=EDIT2 %s < %t.cpp
// EDIT2-NOT: {{^}}#include "foo.h"{{$}}

// RUN: rm -rf %t.dir && mkdir -p %t.dir
// RUN: cp %s %t.cpp
// RUN: echo "[{\"directory\":\"%t.dir\",\"file\":\"../%{t:stem}.tmp.cpp\",\"command\":\":clang++ -I%S/Inputs/ ../%{t:stem}.tmp.cpp\"}]" | sed -e 's/\\/\\\\/g' > %t.dir/compile_commands.json
// RUN: pushd %t.dir
// RUN: clang-include-cleaner -p %{t:stem}.tmp.dir -edit ../%{t:stem}.tmp.cpp
// RUN: popd
// RUN: FileCheck --match-full-lines --check-prefix=EDIT3 %s < %t.cpp
// EDIT3: #include "foo.h"
// EDIT3-NOT: {{^}}#include "foobar.h"{{$}}
58 changes: 51 additions & 7 deletions clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,11 @@ class Action : public clang::ASTFrontendAction {
if (!HTMLReportPath.empty())
writeHTML();

llvm::StringRef Path =
SM.getFileEntryRefForID(SM.getMainFileID())->getName();
assert(!Path.empty() && "Main file path not known?");
// Source File's path of compiler invocation, converted to absolute path.
llvm::SmallString<256> AbsPath(
SM.getFileEntryRefForID(SM.getMainFileID())->getName());
assert(!AbsPath.empty() && "Main file path not known?");
SM.getFileManager().makeAbsolutePath(AbsPath);
llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());

auto Results =
Expand All @@ -185,7 +187,7 @@ class Action : public clang::ASTFrontendAction {
Results.Missing.clear();
if (!Remove)
Results.Unused.clear();
std::string Final = fixIncludes(Results, Path, Code, getStyle(Path));
std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));

if (Print.getNumOccurrences()) {
switch (Print) {
Expand All @@ -202,7 +204,7 @@ class Action : public clang::ASTFrontendAction {
}

if (!Results.Missing.empty() || !Results.Unused.empty())
EditedFiles.try_emplace(Path, Final);
EditedFiles.try_emplace(AbsPath, Final);
}

void writeHTML() {
Expand Down Expand Up @@ -305,8 +307,46 @@ int main(int argc, const char **argv) {
}
}

clang::tooling::ClangTool Tool(OptionsParser->getCompilations(),
OptionsParser->getSourcePathList());
auto VFS = llvm::vfs::getRealFileSystem();
auto &CDB = OptionsParser->getCompilations();
// CDBToAbsPaths is a map from the path in the compilation database to the
// writable absolute path of the file.
std::map<std::string, std::string> CDBToAbsPaths;
// if Edit is enabled, `Factory.editedFiles()` will contain the final code,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also drop the mention of Edit from the comment?

// along with the path given in the compilation database. That path can be
// absolute or relative, and if it is relative, it is relative to the
// "Directory" field in the compilation database. We need to make it
// absolute to write the final code to the correct path.
for (auto &Source : OptionsParser->getSourcePathList()) {
llvm::SmallString<256> AbsPath(Source);
if (auto Err = VFS->makeAbsolute(AbsPath)) {
llvm::errs() << "Failed to get absolute path for " << Source << " : "
<< Err.message() << '\n';
return 1;
}
std::vector<clang::tooling::CompileCommand> Cmds =
CDB.getCompileCommands(AbsPath);
if (Cmds.empty()) {
// Try with the original path.
Cmds = CDB.getCompileCommands(Source);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fallback is still not needed. if we can't find the flags with absolute path, it's fine to bail out.

if (Cmds.empty()) {
// It should be found in the compilation database, even user didn't
// specify the compilation database, the `FixedCompilationDatabase` will
// create an entry from the arguments. So it is an error if we can't
// find the compile commands.
llvm::errs() << "No compile commands found for " << Source << '\n';
return 1;
}
}
for (const auto &Cmd : Cmds) {
llvm::SmallString<256> CDBPath(Cmd.Filename);
std::string Directory(Cmd.Directory);
llvm::sys::fs::make_absolute(Cmd.Directory, CDBPath);
CDBToAbsPaths[std::string(CDBPath)] = std::string(AbsPath);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can extract all of this logic to a helper function:

llvm::Expected<std::map<...>> mapInputsToAbsPaths(...);
...
int main(...) {
  ...
  auto CDBToAbsPaths = mapInputsToAbsPaths(CDB, Opts->OptionsParser->getSourcePathList());
  if (!CDBToAbsPaths) {
    return 1;
  }
  ...
}


clang::tooling::ClangTool Tool(CDB, OptionsParser->getSourcePathList());

auto HeaderFilter = headerFilter();
if (!HeaderFilter)
Expand All @@ -316,6 +356,10 @@ int main(int argc, const char **argv) {
if (Edit) {
for (const auto &NameAndContent : Factory.editedFiles()) {
llvm::StringRef FileName = NameAndContent.first();
if (auto It = CDBToAbsPaths.find(FileName.str());
It != CDBToAbsPaths.end())
FileName = It->second;

const std::string &FinalCode = NameAndContent.second;
if (auto Err = llvm::writeToOutput(
FileName, [&](llvm::raw_ostream &OS) -> llvm::Error {
Expand Down
Loading