[Tooling] Fix misleading progress report when files have multiple compile commands #169640

zeyi2 · 2025-11-26T11:27:08Z

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

…pile commands

llvmbot · 2025-11-26T11:27:40Z

@llvm/pr-subscribers-clang

Author: mitchell (zeyi2)

Changes

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].

Full diff: https://github.com/llvm/llvm-project/pull/169640.diff

1 Files Affected:

(modified) clang/lib/Tooling/Tooling.cpp (+16-11)

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);

zeyi2 · 2025-11-26T11:36:26Z

I believe this patch needs some discussion, for it introduces a new format:

[1/5] (1/1) Processing file ...
[2/5] (1/2) Processing file ...
[2/5] (2/2) Processing file ...
[3/5] (1/2) Processing file ...

I initially considered (and implemented) a solution that does normal printing like [n/TotalActions]. But I discarded it and chose this way of printing due to two main reasons:

Complexity: Calculating the exact number of compile actions requires calling getCompileCommands for every input files. Which is O(N) and may cause startup delay for large projects.
API usage: getCompileCommands may be stateful. Pre-scanning the database violates the requirement of "this method needs to run right before we invoke the tool", and it may trigger unnecessary side effects.

The current patch maintains the O(1) complexity and doesn't violate the API usage, while still reflect the correct progress without misleading users.

I understand that this may not be an ideal way of solving this problem. I highly value any thoughts or feedback on this implementation or potential alternative strategies.

Thanks in advance.

zwuis · 2025-11-27T03:04:47Z

[1/5] (1/1) Processing file ...
[2/5] (1/2) Processing file ...
[2/5] (2/2) Processing file ...
[3/5] (1/2) Processing file ...

It seems that removing (1/1) ((X/Y), Y is 1) can reduce confusion.

zeyi2 · 2025-11-27T10:20:00Z

Thanks for feedback! I've changed the printing to:

[1/9] Processing file ...
[2/9] (1/2) Processing file ...
[2/9] (2/2) Processing file ...
[3/9] Processing file ...
[4/9] Processing file ...

zeyi2 · 2025-12-02T14:05:54Z

Friendly ping, please take a look at this small PR when you have a time :)

Thanks!

vbvictor

Can we make any tests for this new format?
If the user can see it - we should be able to cover it, could be test in clang-tidy infrastructure/ (test different scenarios).

zeyi2 · 2025-12-07T12:22:47Z

Looks like there is a path separators issue. I'm trying to fix it, will ping for another review after the fix is done :)

zeyi2 · 2025-12-09T21:17:15Z

The failed testcase seems unrelated to this PR, I'll try rebase onto main again later.

Fixed now.

github-actions · 2025-12-09T21:17:33Z

🐧 Linux x64 Test Results

111859 tests passed
4506 tests skipped

✅ The build succeeded and all tests passed.

github-actions · 2025-12-09T21:17:33Z

🪟 Windows x64 Test Results

52932 tests passed
2068 tests skipped

✅ The build succeeded and all tests passed.

clang/lib/Tooling/Tooling.cpp

vbvictor · 2025-12-10T15:09:25Z

I didn't find an immediate maintainer of core tooling files; could you look at git log to see who reviewed/committed in these files.

zeyi2 · 2025-12-10T16:43:14Z

Hi, Aaron.

Could you please review this patch? It fixes a misleading progress report issue in Tooling (e.g. [22/18]) when files have multiple compile commands.

I have addressed previous feedback from vbvictor and zwuis, could you do some furthur reviews?

Thanks for your time.

zeyi2 · 2025-12-10T16:45:41Z

The failed testcase seems unrelated to this PR (again), I'll try rebase onto main later.

AaronBallman

The changes seem pretty reasonable but there's a fair amount of unrelated formatting changes. It'd be easier on reviewers to back those unrelated changes out so we can more easily see just what's been materially modified.

zeyi2 · 2025-12-10T17:16:12Z

It'd be easier on reviewers to back those unrelated changes out so we can more easily see just what's been materially modified.

Sorry for the noise, I've cleanup the modification in ToolingTest.cpp. Please take another look when you have time. Thanks for your time and patience.

AaronBallman

LGTM, thank you!

[Tooling] Fix misleading progress report when files have multiple com…

0bcc0a0

…pile commands

llvmbot added the clang Clang issues not falling into any other category label Nov 26, 2025

zeyi2 requested a review from hokein November 26, 2025 11:41

zwuis added the clang:tooling LibTooling label Nov 27, 2025

Fix printing format

3df57de

Remove header

9ee5bf8

vbvictor reviewed Dec 2, 2025

View reviewed changes

Add testcases

32238aa

Fix test on Windows

5c469cd

zeyi2 requested a review from vbvictor December 7, 2025 14:33

zeyi2 added 3 commits December 10, 2025 04:39

cleanup

bcc74bb

reduce noise

2e0cda0

Merge branch 'main' into fix-misreport-progress

afb13cc

Merge branch 'main' into fix-misreport-progress

2b76709

vbvictor reviewed Dec 10, 2025

View reviewed changes

clang/lib/Tooling/Tooling.cpp Outdated Show resolved Hide resolved

zeyi2 requested a review from AaronBallman December 10, 2025 16:36

AaronBallman reviewed Dec 10, 2025

View reviewed changes

Fix format

1df1da5

zeyi2 force-pushed the fix-misreport-progress branch from 66079d8 to 1df1da5 Compare December 10, 2025 17:14

AaronBallman approved these changes Dec 10, 2025

View reviewed changes

[Tooling] Fix misleading progress report when files have multiple compile commands #169640

Are you sure you want to change the base?

[Tooling] Fix misleading progress report when files have multiple compile commands #169640

Conversation

zeyi2 commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 26, 2025

Uh oh!

zeyi2 commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwuis commented Nov 27, 2025

Uh oh!

zeyi2 commented Nov 27, 2025

Uh oh!

zeyi2 commented Dec 2, 2025

Uh oh!

vbvictor left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeyi2 commented Dec 7, 2025

Uh oh!

zeyi2 commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

github-actions bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

Uh oh!

vbvictor commented Dec 10, 2025

Uh oh!

zeyi2 commented Dec 10, 2025

Uh oh!

zeyi2 commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

zeyi2 commented Dec 10, 2025

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zeyi2 commented Nov 26, 2025 •

edited

Loading

zeyi2 commented Nov 26, 2025 •

edited

Loading

vbvictor left a comment •

edited

Loading

zeyi2 commented Dec 9, 2025 •

edited

Loading

github-actions bot commented Dec 9, 2025 •

edited

Loading

github-actions bot commented Dec 9, 2025 •

edited

Loading

zeyi2 commented Dec 10, 2025 •

edited

Loading