https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/76960
>From 6cc7141f1f182763ccec8a4801d3b866cc839324 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Thu, 4 Jan 2024 14:59:22 +0100 Subject: [PATCH 1/3] [include-cleaner] Fix a race issue when editing multiple files. We have a previous fix https://github.com/llvm/llvm-project/commit/be861b64d94198230d8f9889b17280e3cd215a0a, which snapshots all processing files. It works most of times, the snapshot (InMemoryFileSystem) is based on the file path. The file-path-based lookup can fail in a subtle way for some tricky cases (we encounter it internally), which will result in reading a corrupted header. This is a different fix, we don't modify files on the fly, instead, we write files when the tool finishes analysises for all files. --- .../include-cleaner/tool/IncludeCleaner.cpp | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 30aaee29b9a397..eacdfab57b74f0 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -16,10 +16,10 @@ #include "clang/Tooling/Tooling.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FormatVariadic.h" -#include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Regex.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" @@ -110,14 +110,16 @@ format::FormatStyle getStyle(llvm::StringRef Filename) { class Action : public clang::ASTFrontendAction { public: - Action(llvm::function_ref<bool(llvm::StringRef)> HeaderFilter) - : HeaderFilter(HeaderFilter){}; + Action(llvm::function_ref<bool(llvm::StringRef)> HeaderFilter, + llvm::StringMap<std::string> &FileEdits) + : HeaderFilter(HeaderFilter), EditFiles(FileEdits) {} private: RecordedAST AST; RecordedPP PP; PragmaIncludes PI; llvm::function_ref<bool(llvm::StringRef)> HeaderFilter; + llvm::StringMap<std::string>& EditFiles; bool BeginInvocation(CompilerInstance &CI) override { // We only perform include-cleaner analysis. So we disable diagnostics that @@ -181,17 +183,8 @@ class Action : public clang::ASTFrontendAction { } } - if (Edit && (!Results.Missing.empty() || !Results.Unused.empty())) { - if (auto Err = llvm::writeToOutput( - Path, [&](llvm::raw_ostream &OS) -> llvm::Error { - OS << Final; - return llvm::Error::success(); - })) { - llvm::errs() << "Failed to apply edits to " << Path << ": " - << toString(std::move(Err)) << "\n"; - ++Errors; - } - } + if (!Results.Missing.empty() || !Results.Unused.empty()) + EditFiles.try_emplace(Path, Final); } void writeHTML() { @@ -215,11 +208,15 @@ class ActionFactory : public tooling::FrontendActionFactory { : HeaderFilter(HeaderFilter) {} std::unique_ptr<clang::FrontendAction> create() override { - return std::make_unique<Action>(HeaderFilter); + return std::make_unique<Action>(HeaderFilter, EditFiles); } + const llvm::StringMap<std::string> &getEditFiles() const { return EditFiles; } + private: llvm::function_ref<bool(llvm::StringRef)> HeaderFilter; + // Map from file name to final code with the include edits applied. + llvm::StringMap<std::string> EditFiles; }; std::function<bool(llvm::StringRef)> headerFilter() { @@ -274,21 +271,24 @@ int main(int argc, const char **argv) { clang::tooling::ClangTool Tool(OptionsParser->getCompilations(), OptionsParser->getSourcePathList()); - std::vector<std::unique_ptr<llvm::MemoryBuffer>> Buffers; - for (const auto &File : OptionsParser->getSourcePathList()) { - auto Content = llvm::MemoryBuffer::getFile(File); - if (!Content) { - llvm::errs() << "Error: can't read file '" << File - << "': " << Content.getError().message() << "\n"; - return 1; - } - Buffers.push_back(std::move(Content.get())); - Tool.mapVirtualFile(File, Buffers.back()->getBuffer()); - } auto HeaderFilter = headerFilter(); if (!HeaderFilter) return 1; // error already reported. ActionFactory Factory(HeaderFilter); - return Tool.run(&Factory) || Errors != 0; + auto ErrorCode = Tool.run(&Factory); + if (Edit) { + for (const auto &[FileName, FinalCode] : Factory.getEditFiles()) { + if (auto Err = llvm::writeToOutput( + FileName, [&](llvm::raw_ostream &OS) -> llvm::Error { + OS << FinalCode; + return llvm::Error::success(); + })) { + llvm::errs() << "Failed to apply edits to " << FileName << ": " + << toString(std::move(Err)) << "\n"; + ++Errors; + } + } + } + return ErrorCode || Errors != 0; } >From acace9a6753e8f95a718b8ec7464c465c7687d72 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Fri, 5 Jan 2024 09:35:24 +0100 Subject: [PATCH 2/3] Add comment and fix clang-format. --- .../include-cleaner/tool/IncludeCleaner.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index eacdfab57b74f0..7df1b959a4ae0e 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -111,15 +111,15 @@ format::FormatStyle getStyle(llvm::StringRef Filename) { class Action : public clang::ASTFrontendAction { public: Action(llvm::function_ref<bool(llvm::StringRef)> HeaderFilter, - llvm::StringMap<std::string> &FileEdits) - : HeaderFilter(HeaderFilter), EditFiles(FileEdits) {} + llvm::StringMap<std::string> &EditedFiles) + : HeaderFilter(HeaderFilter), EditedFiles(EditedFiles) {} private: RecordedAST AST; RecordedPP PP; PragmaIncludes PI; llvm::function_ref<bool(llvm::StringRef)> HeaderFilter; - llvm::StringMap<std::string>& EditFiles; + llvm::StringMap<std::string> &EditedFiles; bool BeginInvocation(CompilerInstance &CI) override { // We only perform include-cleaner analysis. So we disable diagnostics that @@ -184,7 +184,7 @@ class Action : public clang::ASTFrontendAction { } if (!Results.Missing.empty() || !Results.Unused.empty()) - EditFiles.try_emplace(Path, Final); + EditedFiles.try_emplace(Path, Final); } void writeHTML() { @@ -208,15 +208,15 @@ class ActionFactory : public tooling::FrontendActionFactory { : HeaderFilter(HeaderFilter) {} std::unique_ptr<clang::FrontendAction> create() override { - return std::make_unique<Action>(HeaderFilter, EditFiles); + return std::make_unique<Action>(HeaderFilter, EditedFiles); } - const llvm::StringMap<std::string> &getEditFiles() const { return EditFiles; } + const llvm::StringMap<std::string> &editedFiles() const { return EditedFiles; } private: llvm::function_ref<bool(llvm::StringRef)> HeaderFilter; // Map from file name to final code with the include edits applied. - llvm::StringMap<std::string> EditFiles; + llvm::StringMap<std::string> EditedFiles; }; std::function<bool(llvm::StringRef)> headerFilter() { @@ -278,7 +278,7 @@ int main(int argc, const char **argv) { ActionFactory Factory(HeaderFilter); auto ErrorCode = Tool.run(&Factory); if (Edit) { - for (const auto &[FileName, FinalCode] : Factory.getEditFiles()) { + for (const auto &[FileName, FinalCode] : Factory.editedFiles()) { if (auto Err = llvm::writeToOutput( FileName, [&](llvm::raw_ostream &OS) -> llvm::Error { OS << FinalCode; >From 62bf279a1cfebf6fc779f5fdabaf5eacabcb96d6 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Fri, 5 Jan 2024 09:53:01 +0100 Subject: [PATCH 3/3] clang-format --- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 7df1b959a4ae0e..412b718a9c4660 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -211,7 +211,9 @@ class ActionFactory : public tooling::FrontendActionFactory { return std::make_unique<Action>(HeaderFilter, EditedFiles); } - const llvm::StringMap<std::string> &editedFiles() const { return EditedFiles; } + const llvm::StringMap<std::string> &editedFiles() const { + return EditedFiles; + } private: llvm::function_ref<bool(llvm::StringRef)> HeaderFilter; @@ -278,7 +280,9 @@ int main(int argc, const char **argv) { ActionFactory Factory(HeaderFilter); auto ErrorCode = Tool.run(&Factory); if (Edit) { - for (const auto &[FileName, FinalCode] : Factory.editedFiles()) { + for (const auto & NameAndContent: Factory.editedFiles()) { + llvm::StringRef FileName = NameAndContent.first(); + const std::string& FinalCode = NameAndContent.second; if (auto Err = llvm::writeToOutput( FileName, [&](llvm::raw_ostream &OS) -> llvm::Error { OS << FinalCode; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
