https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/169640
>From 0bcc0a061e4c03f85a2f7481591e97779785f731 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Mon, 24 Nov 2025 10:14:58 +0800 Subject: [PATCH 1/5] [Tooling] Fix misleading progress report when files have multiple compile commands --- clang/lib/Tooling/Tooling.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 9bae12454d2dc..491294b731e85 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -37,6 +37,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/ADT/Twine.h" #include "llvm/Option/ArgList.h" #include "llvm/Option/OptTable.h" @@ -96,7 +97,7 @@ static bool ignoreExtraCC1Commands(const driver::Compilation *Compilation) { OffloadCompilation = true; if (Jobs.size() > 1) { - for (auto *A : Actions){ + for (auto *A : Actions) { // On MacOSX real actions may end up being wrapped in BindArchAction if (isa<driver::BindArchAction>(A)) A = *A->input_begin(); @@ -413,8 +414,8 @@ bool ToolInvocation::run() { Driver->BuildCompilation(llvm::ArrayRef(Argv))); if (!Compilation) return false; - const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments( - &*Diagnostics, Compilation.get()); + const llvm::opt::ArgStringList *const CC1Args = + getCC1Arguments(&*Diagnostics, Compilation.get()); if (!CC1Args) return false; std::unique_ptr<CompilerInvocation> Invocation( @@ -497,9 +498,7 @@ void ClangTool::appendArgumentsAdjuster(ArgumentsAdjuster Adjuster) { ArgsAdjuster = combineAdjusters(std::move(ArgsAdjuster), std::move(Adjuster)); } -void ClangTool::clearArgumentsAdjusters() { - ArgsAdjuster = nullptr; -} +void ClangTool::clearArgumentsAdjusters() { ArgsAdjuster = nullptr; } static void injectResourceDir(CommandLineArguments &Args, const char *Argv0, void *MainAddr) { @@ -555,8 +554,9 @@ int ClangTool::run(ToolAction *Action) { } size_t NumOfTotalFiles = AbsolutePaths.size(); - unsigned ProcessedFileCounter = 0; + unsigned CurrentFileIndex = 0; for (llvm::StringRef File : AbsolutePaths) { + ++CurrentFileIndex; // Increment file counter once per file. // Currently implementations of CompilationDatabase::getCompileCommands can // change the state of the file system (e.g. prepare generated headers), so // this method needs to run right before we invoke the tool, as the next @@ -571,6 +571,7 @@ int ClangTool::run(ToolAction *Action) { FileSkipped = true; continue; } + unsigned CurrentCommandIndexForFile = 0; for (CompileCommand &CompileCommand : CompileCommandsForFile) { // If the 'directory' field of the compilation database is empty, display // an error and use the working directory instead. @@ -617,12 +618,16 @@ int ClangTool::run(ToolAction *Action) { // pass in made-up names here. Make sure this works on other platforms. injectResourceDir(CommandLine, "clang_tool", &StaticSymbol); + ++CurrentCommandIndexForFile; + // FIXME: We need a callback mechanism for the tool writer to output a // customized message for each file. - if (NumOfTotalFiles > 1) - llvm::errs() << "[" + std::to_string(++ProcessedFileCounter) + "/" + - std::to_string(NumOfTotalFiles) + - "] Processing file " + File + if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) + llvm::errs() << "[" + std::to_string(CurrentFileIndex) + "/" + + std::to_string(NumOfTotalFiles) + "] (" + + std::to_string(CurrentCommandIndexForFile) + "/" + + std::to_string(CompileCommandsForFile.size()) + + ") Processing file " + File << ".\n"; ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(), PCHContainerOps); >From 3df57dec9880788efb6dbd50892fdf5f8a0d3b79 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Thu, 27 Nov 2025 18:14:10 +0800 Subject: [PATCH 2/5] Fix printing format --- clang/lib/Tooling/Tooling.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 491294b731e85..2914da9a21888 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -556,7 +556,7 @@ int ClangTool::run(ToolAction *Action) { size_t NumOfTotalFiles = AbsolutePaths.size(); unsigned CurrentFileIndex = 0; for (llvm::StringRef File : AbsolutePaths) { - ++CurrentFileIndex; // Increment file counter once per file. + ++CurrentFileIndex; // Currently implementations of CompilationDatabase::getCompileCommands can // change the state of the file system (e.g. prepare generated headers), so // this method needs to run right before we invoke the tool, as the next @@ -622,13 +622,17 @@ int ClangTool::run(ToolAction *Action) { // FIXME: We need a callback mechanism for the tool writer to output a // customized message for each file. - if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) + if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) { llvm::errs() << "[" + std::to_string(CurrentFileIndex) + "/" + - std::to_string(NumOfTotalFiles) + "] (" + - std::to_string(CurrentCommandIndexForFile) + "/" + - std::to_string(CompileCommandsForFile.size()) + - ") Processing file " + File - << ".\n"; + std::to_string(NumOfTotalFiles) + "]"; + if (CompileCommandsForFile.size() > 1) { + llvm::errs() << " (" + std::to_string(CurrentCommandIndexForFile) + + "/" + + std::to_string(CompileCommandsForFile.size()) + + ")"; + } + llvm::errs() << " Processing file " + File << ".\n"; + } ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(), PCHContainerOps); Invocation.setDiagnosticConsumer(DiagConsumer); >From 9ee5bf8a9fa8d7227a65d3be9da602958c9cd0e7 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Fri, 28 Nov 2025 15:55:14 +0800 Subject: [PATCH 3/5] Remove header --- clang/lib/Tooling/Tooling.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 2914da9a21888..1e5c71da960e8 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -37,7 +37,6 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringRef.h" -#include "llvm/ADT/StringSet.h" #include "llvm/ADT/Twine.h" #include "llvm/Option/ArgList.h" #include "llvm/Option/OptTable.h" >From 32238aaf19a6e7bd9045b6e098c227ecbce478a1 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sun, 7 Dec 2025 19:21:27 +0800 Subject: [PATCH 4/5] Add testcases --- clang/unittests/Tooling/ToolingTest.cpp | 188 ++++++++++++++++++++++++ 1 file changed, 188 insertions(+) diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 25e1d67eb2294..4fe4598c89a81 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -20,6 +20,7 @@ #include "clang/Testing/CommandLineArgs.h" #include "clang/Tooling/ArgumentsAdjusters.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/JSONCompilationDatabase.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Path.h" @@ -1034,5 +1035,192 @@ TEST(runToolOnCode, TestResetDiagnostics) { "void func() { long x; Foo f(x); }")); } +TEST(ClangToolTest, ProgressReportSingleFile) { + SmallString<32> BaseDir; + llvm::sys::path::system_temp_directory(false, BaseDir); + llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix); + std::string BaseDirStr(BaseDir); + + std::string File = BaseDirStr + "/test.cpp"; + + std::string ErrorMessage; + std::string JSONDatabase = R"json([ + { + "directory": "%DIR%", + "command": "clang++ -c test.cpp", + "file": "test.cpp" + }])json"; + + { + size_t Pos = JSONDatabase.find("%DIR%"); + ASSERT_NE(Pos, std::string::npos); + JSONDatabase.replace(Pos, 5, BaseDirStr); + } + + std::unique_ptr<CompilationDatabase> Database( + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)); + ASSERT_TRUE(Database); + + ClangTool Tool(*Database, {File}); + Tool.mapVirtualFile(File, "int x;"); + + testing::internal::CaptureStderr(); + Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); + std::string Output = testing::internal::GetCapturedStderr(); + + EXPECT_TRUE(Output.empty()); +} + +TEST(ClangToolTest, ProgressReportMultipleFiles) { + SmallString<32> BaseDir; + llvm::sys::path::system_temp_directory(false, BaseDir); + llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix); + std::string BaseDirStr(BaseDir); + + std::string File1 = BaseDirStr + "/test1.cpp"; + std::string File2 = BaseDirStr + "/test2.cpp"; + + std::string ErrorMessage; + std::string JSONDatabase = R"json([ + { + "directory": "%DIR%", + "command": "clang++ -c test1.cpp", + "file": "test1.cpp" + }, + { + "directory": "%DIR%", + "command": "clang++ -c test2.cpp", + "file": "test2.cpp" + }])json"; + + for (size_t Pos = 0; + (Pos = JSONDatabase.find("%DIR%", Pos)) != std::string::npos;) { + JSONDatabase.replace(Pos, 5, BaseDirStr); + Pos += BaseDirStr.size(); + } + + std::unique_ptr<CompilationDatabase> Database( + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)); + ASSERT_TRUE(Database); + + ClangTool Tool(*Database, {File1, File2}); + Tool.mapVirtualFile(File1, "int x;"); + Tool.mapVirtualFile(File2, "int y;"); + + testing::internal::CaptureStderr(); + Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); + std::string Output = testing::internal::GetCapturedStderr(); + + std::string Expected = "[1/2] Processing file " + File1 + ".\n" + + "[2/2] Processing file " + File2 + ".\n"; + EXPECT_EQ(Output, Expected); +} + +TEST(ClangToolTest, ProgressReportMultipleCommands) { + SmallString<32> BaseDir; + llvm::sys::path::system_temp_directory(false, BaseDir); + llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix); + std::string BaseDirStr(BaseDir); + + std::string File = BaseDirStr + "/test.cpp"; + + std::string ErrorMessage; + std::string JSONDatabase = R"json([ + { + "directory": "%DIR%", + "command": "clang++ -c test.cpp -DCMD1", + "file": "test.cpp" + }, + { + "directory": "%DIR%", + "command": "clang++ -c test.cpp -DCMD2", + "file": "test.cpp" + }])json"; + + for (size_t Pos = 0; + (Pos = JSONDatabase.find("%DIR%", Pos)) != std::string::npos;) { + JSONDatabase.replace(Pos, 5, BaseDirStr); + Pos += BaseDirStr.size(); + } + + std::unique_ptr<CompilationDatabase> Database( + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)); + ASSERT_TRUE(Database); + + ClangTool Tool(*Database, {File}); + Tool.mapVirtualFile(File, "int x;"); + + testing::internal::CaptureStderr(); + Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); + std::string Output = testing::internal::GetCapturedStderr(); + + std::string Expected = "[1/1] (1/2) Processing file " + File + ".\n" + + "[1/1] (2/2) Processing file " + File + ".\n"; + EXPECT_EQ(Output, Expected); +} + +TEST(ClangToolTest, ProgressReportMixed) { + SmallString<32> BaseDir; + llvm::sys::path::system_temp_directory(false, BaseDir); + llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix); + std::string BaseDirStr(BaseDir); + + std::string File1 = BaseDirStr + "/test1.cpp"; + std::string File2 = BaseDirStr + "/test2.cpp"; + std::string File3 = BaseDirStr + "/test3.cpp"; + + std::string ErrorMessage; + std::string JSONDatabase = R"json([ + { + "directory": "%DIR%", + "command": "clang++ -c test1.cpp", + "file": "test1.cpp" + }, + { + "directory": "%DIR%", + "command": "clang++ -c test2.cpp -DCMD1", + "file": "test2.cpp" + }, + { + "directory": "%DIR%", + "command": "clang++ -c test2.cpp -DCMD2", + "file": "test2.cpp" + }, + { + "directory": "%DIR%", + "command": "clang++ -c test3.cpp", + "file": "test3.cpp" + }])json"; + + for (size_t Pos = 0; + (Pos = JSONDatabase.find("%DIR%", Pos)) != std::string::npos;) { + JSONDatabase.replace(Pos, 5, BaseDirStr); + Pos += BaseDirStr.size(); + } + + std::unique_ptr<CompilationDatabase> Database( + JSONCompilationDatabase::loadFromBuffer(JSONDatabase, ErrorMessage, + JSONCommandLineSyntax::Gnu)); + ASSERT_TRUE(Database); + + ClangTool Tool(*Database, {File1, File2, File3}); + Tool.mapVirtualFile(File1, "int x;"); + Tool.mapVirtualFile(File2, "int y;"); + Tool.mapVirtualFile(File3, "int z;"); + + testing::internal::CaptureStderr(); + Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); + std::string Output = testing::internal::GetCapturedStderr(); + + std::string Expected = "[1/3] Processing file " + File1 + ".\n" + + "[2/3] (1/2) Processing file " + File2 + ".\n" + + "[2/3] (2/2) Processing file " + File2 + ".\n" + + "[3/3] Processing file " + File3 + ".\n"; + EXPECT_EQ(Output, Expected); +} + } // end namespace tooling } // end namespace clang >From 5c469cd476e56ceaefbed581737dec6b51fc119d Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sun, 7 Dec 2025 22:01:28 +0800 Subject: [PATCH 5/5] Fix test on Windows --- clang/unittests/Tooling/ToolingTest.cpp | 31 ++++++++++++++++++------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 4fe4598c89a81..bb33f98a1ff23 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -1113,8 +1113,13 @@ TEST(ClangToolTest, ProgressReportMultipleFiles) { Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); std::string Output = testing::internal::GetCapturedStderr(); - std::string Expected = "[1/2] Processing file " + File1 + ".\n" + - "[2/2] Processing file " + File2 + ".\n"; + SmallString<32> NativeFile1(File1); + llvm::sys::path::native(NativeFile1); + SmallString<32> NativeFile2(File2); + llvm::sys::path::native(NativeFile2); + std::string Expected = "[1/2] Processing file " + std::string(NativeFile1) + + ".\n" + "[2/2] Processing file " + + std::string(NativeFile2) + ".\n"; EXPECT_EQ(Output, Expected); } @@ -1157,8 +1162,11 @@ TEST(ClangToolTest, ProgressReportMultipleCommands) { Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); std::string Output = testing::internal::GetCapturedStderr(); - std::string Expected = "[1/1] (1/2) Processing file " + File + ".\n" + - "[1/1] (2/2) Processing file " + File + ".\n"; + SmallString<32> NativeFile(File); + llvm::sys::path::native(NativeFile); + std::string Expected = + "[1/1] (1/2) Processing file " + std::string(NativeFile) + ".\n" + + "[1/1] (2/2) Processing file " + std::string(NativeFile) + ".\n"; EXPECT_EQ(Output, Expected); } @@ -1215,10 +1223,17 @@ TEST(ClangToolTest, ProgressReportMixed) { Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); std::string Output = testing::internal::GetCapturedStderr(); - std::string Expected = "[1/3] Processing file " + File1 + ".\n" + - "[2/3] (1/2) Processing file " + File2 + ".\n" + - "[2/3] (2/2) Processing file " + File2 + ".\n" + - "[3/3] Processing file " + File3 + ".\n"; + SmallString<32> NativeFile1(File1); + llvm::sys::path::native(NativeFile1); + SmallString<32> NativeFile2(File2); + llvm::sys::path::native(NativeFile2); + SmallString<32> NativeFile3(File3); + llvm::sys::path::native(NativeFile3); + std::string Expected = + "[1/3] Processing file " + std::string(NativeFile1) + ".\n" + + "[2/3] (1/2) Processing file " + std::string(NativeFile2) + ".\n" + + "[2/3] (2/2) Processing file " + std::string(NativeFile2) + ".\n" + + "[3/3] Processing file " + std::string(NativeFile3) + ".\n"; EXPECT_EQ(Output, Expected); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
