Author: mitchell Date: 2025-12-11T11:12:47+08:00 New Revision: 2614af08c1875e7928c6036237e434943730631a
URL: https://github.com/llvm/llvm-project/commit/2614af08c1875e7928c6036237e434943730631a DIFF: https://github.com/llvm/llvm-project/commit/2614af08c1875e7928c6036237e434943730631a.diff LOG: [Tooling] Fix misleading progress report when files have multiple compile commands (#169640) This patch fixes an issue in progress reporting where the processed item counter could exceed the total item count, leading to confusing outputs like [22/18]. Closes [#169168](https://github.com/llvm/llvm-project/issues/169168) Added: Modified: clang/lib/Tooling/Tooling.cpp clang/unittests/Tooling/ToolingTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 1d55f615de8a9..46b2cc1ac99c1 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -97,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(); @@ -414,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( @@ -498,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 +553,9 @@ int ClangTool::run(ToolAction *Action) { } size_t NumOfTotalFiles = AbsolutePaths.size(); - unsigned ProcessedFileCounter = 0; + unsigned CurrentFileIndex = 0; for (llvm::StringRef File : AbsolutePaths) { + ++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 @@ -571,6 +570,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,13 +617,20 @@ 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 - << ".\n"; + if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) { + llvm::errs() << "[" << std::to_string(CurrentFileIndex) << "/" + << 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); diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 25e1d67eb2294..9a7559405c43c 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -20,8 +20,10 @@ #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/JSON.h" #include "llvm/Support/Path.h" #include "llvm/Support/TargetSelect.h" #include "llvm/TargetParser/Host.h" @@ -1034,5 +1036,136 @@ TEST(runToolOnCode, TestResetDiagnostics) { "void func() { long x; Foo f(x); }")); } +namespace { +struct TestCommand { + llvm::StringRef File; + llvm::StringRef Command; +}; + +std::string runToolWithProgress(llvm::ArrayRef<TestCommand> Commands, + llvm::StringRef BaseDir) { + std::string ErrorMessage; + + llvm::json::Array Entries; + for (const auto &Cmd : Commands) { + Entries.push_back(llvm::json::Object{ + {"directory", BaseDir}, {"command", Cmd.Command}, {"file", Cmd.File}}); + } + std::string DatabaseContent; + llvm::raw_string_ostream OS(DatabaseContent); + OS << llvm::json::Value(std::move(Entries)); + + std::unique_ptr<CompilationDatabase> Database( + JSONCompilationDatabase::loadFromBuffer(DatabaseContent, ErrorMessage, + JSONCommandLineSyntax::Gnu)); + if (!Database) { + ADD_FAILURE() << "Failed to load compilation database: " << ErrorMessage; + return ""; + } + + std::vector<std::string> AbsoluteFiles; + for (const auto &Cmd : Commands) { + SmallString<32> NativeFile(BaseDir); + llvm::sys::path::append(NativeFile, Cmd.File); + llvm::sys::path::native(NativeFile); + std::string AbsPath = std::string(NativeFile); + if (AbsoluteFiles.empty() || AbsoluteFiles.back() != AbsPath) { + AbsoluteFiles.push_back(AbsPath); + } + } + + ClangTool Tool(*Database, AbsoluteFiles); + for (const auto &F : AbsoluteFiles) { + Tool.mapVirtualFile(F, "int x;"); + } + + testing::internal::CaptureStderr(); + Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get()); + return testing::internal::GetCapturedStderr(); +} +} // namespace + +TEST(ClangToolTest, ProgressReportSingleFile) { + SmallString<32> BaseDir; + llvm::sys::path::system_temp_directory(false, BaseDir); + llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix); + + EXPECT_TRUE( + runToolWithProgress({{"test.cpp", "clang++ -c test.cpp"}}, BaseDir) + .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 Output = + runToolWithProgress({{"test1.cpp", "clang++ -c test1.cpp"}, + {"test2.cpp", "clang++ -c test2.cpp"}}, + BaseDir); + + SmallString<32> NativeFile1(BaseDir); + llvm::sys::path::append(NativeFile1, "test1.cpp"); + llvm::sys::path::native(NativeFile1); + SmallString<32> NativeFile2(BaseDir); + llvm::sys::path::append(NativeFile2, "test2.cpp"); + 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); +} + +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 Output = + runToolWithProgress({{"test.cpp", "clang++ -c test.cpp -DCMD1"}, + {"test.cpp", "clang++ -c test.cpp -DCMD2"}}, + BaseDir); + + SmallString<32> NativeFile(BaseDir); + llvm::sys::path::append(NativeFile, "test.cpp"); + 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); +} + +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 Output = + runToolWithProgress({{"test1.cpp", "clang++ -c test1.cpp"}, + {"test2.cpp", "clang++ -c test2.cpp -DCMD1"}, + {"test2.cpp", "clang++ -c test2.cpp -DCMD2"}, + {"test3.cpp", "clang++ -c test3.cpp"}}, + BaseDir); + + SmallString<32> NativeFile1(BaseDir); + llvm::sys::path::append(NativeFile1, "test1.cpp"); + llvm::sys::path::native(NativeFile1); + SmallString<32> NativeFile2(BaseDir); + llvm::sys::path::append(NativeFile2, "test2.cpp"); + llvm::sys::path::native(NativeFile2); + SmallString<32> NativeFile3(BaseDir); + llvm::sys::path::append(NativeFile3, "test3.cpp"); + 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); +} + } // end namespace tooling } // end namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
