https://github.com/bc-lee updated https://github.com/llvm/llvm-project/pull/111375
>From 23b90bba12c010e5882e09e9f6b765a7281324aa Mon Sep 17 00:00:00 2001 From: Byoungchan Lee <byoungchan....@gmx.com> Date: Mon, 7 Oct 2024 22:19:38 +0900 Subject: [PATCH 1/2] [clang-include-cleaner] Fix incorrect directory issue for writing files 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 checking whether the input file path is absolute. If it isn't, the path is concatenated with the directory of the input file specified in the compilation database. Fixes #110843. --- .../include-cleaner/tool/IncludeCleaner.cpp | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 080099adc9a07a..1cb8b9c4ae99b7 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -173,11 +173,22 @@ class Action : public clang::ASTFrontendAction { if (!HTMLReportPath.empty()) writeHTML(); + // Source File's path relative to compilation database. llvm::StringRef Path = SM.getFileEntryRefForID(SM.getMainFileID())->getName(); assert(!Path.empty() && "Main file path not known?"); llvm::StringRef Code = SM.getBufferData(SM.getMainFileID()); + auto Cwd = getCompilerInstance() + .getFileManager() + .getVirtualFileSystem() + .getCurrentWorkingDirectory(); + if (Cwd.getError()) { + llvm::errs() << "Failed to get current working directory: " + << Cwd.getError().message() << "\n"; + return; + } + auto Results = analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, getCompilerInstance().getPreprocessor(), HeaderFilter); @@ -201,8 +212,17 @@ class Action : public clang::ASTFrontendAction { } } - if (!Results.Missing.empty() || !Results.Unused.empty()) - EditedFiles.try_emplace(Path, Final); + if (!Results.Missing.empty() || !Results.Unused.empty()) { + auto Sept = llvm::sys::path::get_separator(); + // Adjust the path to be absolute if it is not. + std::string FullPath; + if (llvm::sys::path::is_absolute(Path)) + FullPath = Path.str(); + else + FullPath = Cwd.get() + Sept.str() + Path.str(); + + EditedFiles.try_emplace(FullPath, Final); + } } void writeHTML() { >From eef397cf22e062cf024ffcd84fef7bdf98833351 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee <byoungchan....@gmx.com> Date: Wed, 9 Oct 2024 19:43:02 +0900 Subject: [PATCH 2/2] Fix path issue identified by @kadircet Some Bazel systems might have a read-only file system in the build directory. In such cases, the output file should be written to the original source file. Adjusted the logic to find the correct output directory based on the compilation database and the current working directory. --- .../include-cleaner/tool/IncludeCleaner.cpp | 95 ++++++++++++++----- 1 file changed, 73 insertions(+), 22 deletions(-) diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 1cb8b9c4ae99b7..4481601140e3fe 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -179,16 +179,6 @@ class Action : public clang::ASTFrontendAction { assert(!Path.empty() && "Main file path not known?"); llvm::StringRef Code = SM.getBufferData(SM.getMainFileID()); - auto Cwd = getCompilerInstance() - .getFileManager() - .getVirtualFileSystem() - .getCurrentWorkingDirectory(); - if (Cwd.getError()) { - llvm::errs() << "Failed to get current working directory: " - << Cwd.getError().message() << "\n"; - return; - } - auto Results = analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, getCompilerInstance().getPreprocessor(), HeaderFilter); @@ -212,17 +202,8 @@ class Action : public clang::ASTFrontendAction { } } - if (!Results.Missing.empty() || !Results.Unused.empty()) { - auto Sept = llvm::sys::path::get_separator(); - // Adjust the path to be absolute if it is not. - std::string FullPath; - if (llvm::sys::path::is_absolute(Path)) - FullPath = Path.str(); - else - FullPath = Cwd.get() + Sept.str() + Path.str(); - - EditedFiles.try_emplace(FullPath, Final); - } + if (!Results.Missing.empty() || !Results.Unused.empty()) + EditedFiles.try_emplace(Path, Final); } void writeHTML() { @@ -300,6 +281,42 @@ std::function<bool(llvm::StringRef)> headerFilter() { }; } +llvm::ErrorOr<std::string> getCurrentWorkingDirectory() { + llvm::SmallString<256> CurrentPath; + if (const std::error_code EC = llvm::sys::fs::current_path(CurrentPath)) + return llvm::ErrorOr<std::string>(EC); + return std::string(CurrentPath.str()); +} + +std::optional<std::string> +adjustCompilationPath(const std::string &Filename, const std::string &Directory, + const std::string &CurrentWorkingDirectory) { + if (llvm::sys::path::is_absolute(Filename)) { + // If the file path is already absolute, use it as is. + return Filename; + } + + auto Sept = llvm::sys::path::get_separator().str(); + // First, try to find the file based on the compilation database. + std::string FilePath = Directory + Sept + Filename; + // Check if it is writable. + if (llvm::sys::fs::access(FilePath, llvm::sys::fs::AccessMode::Write) != + std::error_code()) { + // If not, try to find the file based on the current working + // directory, as some Bazel invocations may not set the compilation + // invocation's filesystem as non-writable. In such cases, we can + // find the file based on the current working directory. + FilePath = + static_cast<std::string>(CurrentWorkingDirectory) + Sept + Filename; + if (llvm::sys::fs::access(FilePath, llvm::sys::fs::AccessMode::Write) != + std::error_code()) { + llvm::errs() << "Failed to find a writable path for " << Filename << "\n"; + return std::nullopt; + } + } + return FilePath; +} + } // namespace } // namespace include_cleaner } // namespace clang @@ -325,7 +342,32 @@ int main(int argc, const char **argv) { } } - clang::tooling::ClangTool Tool(OptionsParser->getCompilations(), + auto &CompilationDatabase = OptionsParser->getCompilations(); + // Mapping of edited file paths to their actual paths. + std::map<std::string, std::string> EditedFilesToActualFiles; + if (Edit) { + std::vector<clang::tooling::CompileCommand> Commands = + CompilationDatabase.getAllCompileCommands(); + auto CurrentWorkingDirectory = getCurrentWorkingDirectory(); + // if (CurrentWorkingDirectory.get() + if (auto EC = CurrentWorkingDirectory.getError()) { + llvm::errs() << "Failed to get current working directory: " + << EC.message() << "\n"; + return 1; + } + + for (const auto &Command : Commands) { + auto AdjustedFilePath = adjustCompilationPath( + Command.Filename, Command.Directory, CurrentWorkingDirectory.get()); + if (!AdjustedFilePath.has_value()) { + // We already printed an error message. + return 1; + } + EditedFilesToActualFiles[Command.Filename] = AdjustedFilePath.value(); + } + } + + clang::tooling::ClangTool Tool(CompilationDatabase, OptionsParser->getSourcePathList()); auto HeaderFilter = headerFilter(); @@ -336,6 +378,15 @@ int main(int argc, const char **argv) { if (Edit) { for (const auto &NameAndContent : Factory.editedFiles()) { llvm::StringRef FileName = NameAndContent.first(); + auto AdjustedFilePathIter = EditedFilesToActualFiles.find(FileName.str()); + if (AdjustedFilePathIter != EditedFilesToActualFiles.end()) { + FileName = AdjustedFilePathIter->second; + } else { + llvm::errs() << "Failed to find the actual path for " << FileName + << "\n"; + ++Errors; + } + 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