Skip to content

Commit 2614af0

Browse files
zeyi2zeyi2
authored
[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](#169168)
1 parent 3fdce79 commit 2614af0

File tree

2 files changed

+152
-12
lines changed

2 files changed

+152
-12
lines changed

‎clang/lib/Tooling/Tooling.cpp‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ static bool ignoreExtraCC1Commands(const driver::Compilation *Compilation) {
9797
OffloadCompilation = true;
9898

9999
if (Jobs.size() > 1) {
100-
for (auto *A : Actions){
100+
for (auto *A : Actions) {
101101
// On MacOSX real actions may end up being wrapped in BindArchAction
102102
if (isa<driver::BindArchAction>(A))
103103
A = *A->input_begin();
@@ -414,8 +414,8 @@ bool ToolInvocation::run() {
414414
Driver->BuildCompilation(llvm::ArrayRef(Argv)));
415415
if (!Compilation)
416416
return false;
417-
const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments(
418-
&*Diagnostics, Compilation.get());
417+
const llvm::opt::ArgStringList *const CC1Args =
418+
getCC1Arguments(&*Diagnostics, Compilation.get());
419419
if (!CC1Args)
420420
return false;
421421
std::unique_ptr<CompilerInvocation> Invocation(
@@ -498,9 +498,7 @@ void ClangTool::appendArgumentsAdjuster(ArgumentsAdjuster Adjuster) {
498498
ArgsAdjuster = combineAdjusters(std::move(ArgsAdjuster), std::move(Adjuster));
499499
}
500500

501-
void ClangTool::clearArgumentsAdjusters() {
502-
ArgsAdjuster = nullptr;
503-
}
501+
void ClangTool::clearArgumentsAdjusters() { ArgsAdjuster = nullptr; }
504502

505503
static void injectResourceDir(CommandLineArguments &Args, const char *Argv0,
506504
void *MainAddr) {
@@ -555,8 +553,9 @@ int ClangTool::run(ToolAction *Action) {
555553
}
556554

557555
size_t NumOfTotalFiles = AbsolutePaths.size();
558-
unsigned ProcessedFileCounter = 0;
556+
unsigned CurrentFileIndex = 0;
559557
for (llvm::StringRef File : AbsolutePaths) {
558+
++CurrentFileIndex;
560559
// Currently implementations of CompilationDatabase::getCompileCommands can
561560
// change the state of the file system (e.g. prepare generated headers), so
562561
// this method needs to run right before we invoke the tool, as the next
@@ -571,6 +570,7 @@ int ClangTool::run(ToolAction *Action) {
571570
FileSkipped = true;
572571
continue;
573572
}
573+
unsigned CurrentCommandIndexForFile = 0;
574574
for (CompileCommand &CompileCommand : CompileCommandsForFile) {
575575
// If the 'directory' field of the compilation database is empty, display
576576
// an error and use the working directory instead.
@@ -617,13 +617,20 @@ int ClangTool::run(ToolAction *Action) {
617617
// pass in made-up names here. Make sure this works on other platforms.
618618
injectResourceDir(CommandLine, "clang_tool", &StaticSymbol);
619619

620+
++CurrentCommandIndexForFile;
621+
620622
// FIXME: We need a callback mechanism for the tool writer to output a
621623
// customized message for each file.
622-
if (NumOfTotalFiles > 1)
623-
llvm::errs() << "[" + std::to_string(++ProcessedFileCounter) + "/" +
624-
std::to_string(NumOfTotalFiles) +
625-
"] Processing file " + File
626-
<< ".\n";
624+
if (NumOfTotalFiles > 1 || CompileCommandsForFile.size() > 1) {
625+
llvm::errs() << "[" << std::to_string(CurrentFileIndex) << "/"
626+
<< std::to_string(NumOfTotalFiles) << "]";
627+
if (CompileCommandsForFile.size() > 1) {
628+
llvm::errs() << " (" << std::to_string(CurrentCommandIndexForFile)
629+
<< "/" << std::to_string(CompileCommandsForFile.size())
630+
<< ")";
631+
}
632+
llvm::errs() << " Processing file " << File << ".\n";
633+
}
627634
ToolInvocation Invocation(std::move(CommandLine), Action, Files.get(),
628635
PCHContainerOps);
629636
Invocation.setDiagnosticConsumer(DiagConsumer);

‎clang/unittests/Tooling/ToolingTest.cpp‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
#include "clang/Testing/CommandLineArgs.h"
2121
#include "clang/Tooling/ArgumentsAdjusters.h"
2222
#include "clang/Tooling/CompilationDatabase.h"
23+
#include "clang/Tooling/JSONCompilationDatabase.h"
2324
#include "llvm/ADT/STLExtras.h"
2425
#include "llvm/ADT/StringRef.h"
26+
#include "llvm/Support/JSON.h"
2527
#include "llvm/Support/Path.h"
2628
#include "llvm/Support/TargetSelect.h"
2729
#include "llvm/TargetParser/Host.h"
@@ -1034,5 +1036,136 @@ TEST(runToolOnCode, TestResetDiagnostics) {
10341036
"void func() { long x; Foo f(x); }"));
10351037
}
10361038

1039+
namespace {
1040+
struct TestCommand {
1041+
llvm::StringRef File;
1042+
llvm::StringRef Command;
1043+
};
1044+
1045+
std::string runToolWithProgress(llvm::ArrayRef<TestCommand> Commands,
1046+
llvm::StringRef BaseDir) {
1047+
std::string ErrorMessage;
1048+
1049+
llvm::json::Array Entries;
1050+
for (const auto &Cmd : Commands) {
1051+
Entries.push_back(llvm::json::Object{
1052+
{"directory", BaseDir}, {"command", Cmd.Command}, {"file", Cmd.File}});
1053+
}
1054+
std::string DatabaseContent;
1055+
llvm::raw_string_ostream OS(DatabaseContent);
1056+
OS << llvm::json::Value(std::move(Entries));
1057+
1058+
std::unique_ptr<CompilationDatabase> Database(
1059+
JSONCompilationDatabase::loadFromBuffer(DatabaseContent, ErrorMessage,
1060+
JSONCommandLineSyntax::Gnu));
1061+
if (!Database) {
1062+
ADD_FAILURE() << "Failed to load compilation database: " << ErrorMessage;
1063+
return "";
1064+
}
1065+
1066+
std::vector<std::string> AbsoluteFiles;
1067+
for (const auto &Cmd : Commands) {
1068+
SmallString<32> NativeFile(BaseDir);
1069+
llvm::sys::path::append(NativeFile, Cmd.File);
1070+
llvm::sys::path::native(NativeFile);
1071+
std::string AbsPath = std::string(NativeFile);
1072+
if (AbsoluteFiles.empty() || AbsoluteFiles.back() != AbsPath) {
1073+
AbsoluteFiles.push_back(AbsPath);
1074+
}
1075+
}
1076+
1077+
ClangTool Tool(*Database, AbsoluteFiles);
1078+
for (const auto &F : AbsoluteFiles) {
1079+
Tool.mapVirtualFile(F, "int x;");
1080+
}
1081+
1082+
testing::internal::CaptureStderr();
1083+
Tool.run(newFrontendActionFactory<SyntaxOnlyAction>().get());
1084+
return testing::internal::GetCapturedStderr();
1085+
}
1086+
} // namespace
1087+
1088+
TEST(ClangToolTest, ProgressReportSingleFile) {
1089+
SmallString<32> BaseDir;
1090+
llvm::sys::path::system_temp_directory(false, BaseDir);
1091+
llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix);
1092+
1093+
EXPECT_TRUE(
1094+
runToolWithProgress({{"test.cpp", "clang++ -c test.cpp"}}, BaseDir)
1095+
.empty());
1096+
}
1097+
1098+
TEST(ClangToolTest, ProgressReportMultipleFiles) {
1099+
SmallString<32> BaseDir;
1100+
llvm::sys::path::system_temp_directory(false, BaseDir);
1101+
llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix);
1102+
1103+
std::string Output =
1104+
runToolWithProgress({{"test1.cpp", "clang++ -c test1.cpp"},
1105+
{"test2.cpp", "clang++ -c test2.cpp"}},
1106+
BaseDir);
1107+
1108+
SmallString<32> NativeFile1(BaseDir);
1109+
llvm::sys::path::append(NativeFile1, "test1.cpp");
1110+
llvm::sys::path::native(NativeFile1);
1111+
SmallString<32> NativeFile2(BaseDir);
1112+
llvm::sys::path::append(NativeFile2, "test2.cpp");
1113+
llvm::sys::path::native(NativeFile2);
1114+
1115+
std::string Expected = "[1/2] Processing file " + std::string(NativeFile1) +
1116+
".\n" + "[2/2] Processing file " +
1117+
std::string(NativeFile2) + ".\n";
1118+
EXPECT_EQ(Output, Expected);
1119+
}
1120+
1121+
TEST(ClangToolTest, ProgressReportMultipleCommands) {
1122+
SmallString<32> BaseDir;
1123+
llvm::sys::path::system_temp_directory(false, BaseDir);
1124+
llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix);
1125+
1126+
std::string Output =
1127+
runToolWithProgress({{"test.cpp", "clang++ -c test.cpp -DCMD1"},
1128+
{"test.cpp", "clang++ -c test.cpp -DCMD2"}},
1129+
BaseDir);
1130+
1131+
SmallString<32> NativeFile(BaseDir);
1132+
llvm::sys::path::append(NativeFile, "test.cpp");
1133+
llvm::sys::path::native(NativeFile);
1134+
std::string Expected =
1135+
"[1/1] (1/2) Processing file " + std::string(NativeFile) + ".\n" +
1136+
"[1/1] (2/2) Processing file " + std::string(NativeFile) + ".\n";
1137+
EXPECT_EQ(Output, Expected);
1138+
}
1139+
1140+
TEST(ClangToolTest, ProgressReportMixed) {
1141+
SmallString<32> BaseDir;
1142+
llvm::sys::path::system_temp_directory(false, BaseDir);
1143+
llvm::sys::path::native(BaseDir, llvm::sys::path::Style::posix);
1144+
1145+
std::string Output =
1146+
runToolWithProgress({{"test1.cpp", "clang++ -c test1.cpp"},
1147+
{"test2.cpp", "clang++ -c test2.cpp -DCMD1"},
1148+
{"test2.cpp", "clang++ -c test2.cpp -DCMD2"},
1149+
{"test3.cpp", "clang++ -c test3.cpp"}},
1150+
BaseDir);
1151+
1152+
SmallString<32> NativeFile1(BaseDir);
1153+
llvm::sys::path::append(NativeFile1, "test1.cpp");
1154+
llvm::sys::path::native(NativeFile1);
1155+
SmallString<32> NativeFile2(BaseDir);
1156+
llvm::sys::path::append(NativeFile2, "test2.cpp");
1157+
llvm::sys::path::native(NativeFile2);
1158+
SmallString<32> NativeFile3(BaseDir);
1159+
llvm::sys::path::append(NativeFile3, "test3.cpp");
1160+
llvm::sys::path::native(NativeFile3);
1161+
1162+
std::string Expected =
1163+
"[1/3] Processing file " + std::string(NativeFile1) + ".\n" +
1164+
"[2/3] (1/2) Processing file " + std::string(NativeFile2) + ".\n" +
1165+
"[2/3] (2/2) Processing file " + std::string(NativeFile2) + ".\n" +
1166+
"[3/3] Processing file " + std::string(NativeFile3) + ".\n";
1167+
EXPECT_EQ(Output, Expected);
1168+
}
1169+
10371170
} // end namespace tooling
10381171
} // end namespace clang

0 commit comments

Comments
 (0)