Author: Byoungchan Lee Date: 2024-10-17T10:16:07+02:00 New Revision: 4cda28c1ada702a08f6960eb4c93919187c1d4d1
URL: https://github.com/llvm/llvm-project/commit/4cda28c1ada702a08f6960eb4c93919187c1d4d1 DIFF: https://github.com/llvm/llvm-project/commit/4cda28c1ada702a08f6960eb4c93919187c1d4d1.diff LOG: [clang-include-cleaner] Fix incorrect directory issue for writing files (#111375) If the current working directory of `clang-include-cleaner` differs from the directory of the input files specified in the compilation database, it doesn't adjust the input file paths properly. As a result, `clang-include-cleaner` either writes files to the wrong directory or fails to write files altogether. This pull request fixes the issue by adjusting the input file paths based on the directory specified in the compilation database. If that directory is not writable, `clang-include-cleaner` will write the output relative to the current working directory. Fixes #110843. Added: Modified: clang-tools-extra/include-cleaner/test/tool.cpp clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/include-cleaner/test/tool.cpp b/clang-tools-extra/include-cleaner/test/tool.cpp index 2155eec189d186..d72d2317ce2b1d 100644 --- a/clang-tools-extra/include-cleaner/test/tool.cpp +++ b/clang-tools-extra/include-cleaner/test/tool.cpp @@ -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"{{$}} diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 080099adc9a07a..6bd9c40c70753c 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -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 = @@ -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) { @@ -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() { @@ -280,6 +282,48 @@ std::function<bool(llvm::StringRef)> headerFilter() { }; } +// Maps absolute path of each files of each compilation commands to the +// absolute path of the input file. +llvm::Expected<std::map<std::string, std::string>> +mapInputsToAbsPaths(clang::tooling::CompilationDatabase &CDB, + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, + const std::vector<std::string> &Inputs) { + std::map<std::string, std::string> CDBToAbsPaths; + // Factory.editedFiles()` will contain the final code, 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 : Inputs) { + llvm::SmallString<256> AbsPath(Source); + if (auto Err = VFS->makeAbsolute(AbsPath)) { + llvm::errs() << "Failed to get absolute path for " << Source << " : " + << Err.message() << '\n'; + return std::move(llvm::errorCodeToError(Err)); + } + std::vector<clang::tooling::CompileCommand> Cmds = + CDB.getCompileCommands(AbsPath); + 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. + std::string ErrorMsg = + llvm::formatv("No compile commands found for {0}", AbsPath).str(); + llvm::errs() << ErrorMsg << '\n'; + return llvm::make_error<llvm::StringError>( + ErrorMsg, llvm::inconvertibleErrorCode()); + } + 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); + } + } + return CDBToAbsPaths; +} + } // namespace } // namespace include_cleaner } // namespace clang @@ -305,8 +349,16 @@ 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. + auto CDBToAbsPaths = + mapInputsToAbsPaths(CDB, VFS, OptionsParser->getSourcePathList()); + if (!CDBToAbsPaths) + return 1; + + clang::tooling::ClangTool Tool(CDB, OptionsParser->getSourcePathList()); auto HeaderFilter = headerFilter(); if (!HeaderFilter) @@ -316,6 +368,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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
