[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 5 11:21:11 PST 2023
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/67839
>From 9c798ed914b0008d98587c94f8ee3bb914412215 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 8 Sep 2023 16:39:10 -0700
Subject: [PATCH 1/2] [clang][tidy] Ensure rewriter has the correct CWD
This reverts commit 65331da0032ab4253a4bc0ddcb2da67664bd86a9.
---
clang-tools-extra/clang-tidy/ClangTidy.cpp | 8 ++++++++
clang/lib/Rewrite/Rewriter.cpp | 13 +++++++------
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 4b1a67b6dd98a..8805864a8537a 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -227,6 +227,14 @@ class ErrorReporter {
llvm::errs() << "Can't apply replacements for file " << File << "\n";
}
}
+
+ auto BuildDir = Context.getCurrentBuildDirectory();
+ if (!BuildDir.empty())
+ Rewrite.getSourceMgr()
+ .getFileManager()
+ .getVirtualFileSystem()
+ .setCurrentWorkingDirectory(BuildDir);
+
if (Rewrite.overwriteChangedFiles()) {
llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
} else {
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index ef2858990dd95..0896221dd0bde 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -412,12 +412,13 @@ bool Rewriter::overwriteChangedFiles() {
unsigned OverwriteFailure = Diag.getCustomDiagID(
DiagnosticsEngine::Error, "unable to overwrite file %0: %1");
for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
- const FileEntry *Entry = getSourceMgr().getFileEntryForID(I->first);
- if (auto Error =
- llvm::writeToOutput(Entry->getName(), [&](llvm::raw_ostream &OS) {
- I->second.write(OS);
- return llvm::Error::success();
- })) {
+ OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first);
+ llvm::SmallString<128> Path(Entry->getName());
+ getSourceMgr().getFileManager().makeAbsolutePath(Path);
+ if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) {
+ I->second.write(OS);
+ return llvm::Error::success();
+ })) {
Diag.Report(OverwriteFailure)
<< Entry->getName() << llvm::toString(std::move(Error));
AllWritten = false;
>From 225e4f20385b9b8890639002f7afd8bb476d738b Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 4 Dec 2023 14:48:58 -0800
Subject: [PATCH 2/2] [clang][tidy] Keep track of CWD, improve test coverage
---
clang-tools-extra/clang-tidy/ClangTidy.cpp | 30 +++++++++++--------
.../Inputs/compilation-database/template.json | 9 ++++--
.../clang-tidy-run-with-database.cpp | 11 ++++---
3 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 8805864a8537a..36ea5a39875e5 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -147,7 +147,8 @@ class ErrorReporter {
Files.makeAbsolutePath(FixAbsoluteFilePath);
tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(),
Repl.getLength(), Repl.getReplacementText());
- Replacements &Replacements = FileReplacements[R.getFilePath()];
+ auto &Entry = FileReplacements[R.getFilePath()];
+ Replacements &Replacements = Entry.Replacements;
llvm::Error Err = Replacements.add(R);
if (Err) {
// FIXME: Implement better conflict handling.
@@ -174,6 +175,7 @@ class ErrorReporter {
}
FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
+ Entry.BuildDir = Error.BuildDirectory;
}
}
}
@@ -189,9 +191,12 @@ class ErrorReporter {
void finish() {
if (TotalFixes > 0) {
- Rewriter Rewrite(SourceMgr, LangOpts);
+ bool AnyNotWritten = false;
for (const auto &FileAndReplacements : FileReplacements) {
+ Rewriter Rewrite(SourceMgr, LangOpts);
StringRef File = FileAndReplacements.first();
+ Files.getVirtualFileSystem().setCurrentWorkingDirectory(
+ FileAndReplacements.second.BuildDir);
llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
SourceMgr.getFileManager().getBufferForFile(File);
if (!Buffer) {
@@ -208,8 +213,8 @@ class ErrorReporter {
continue;
}
llvm::Expected<tooling::Replacements> Replacements =
- format::cleanupAroundReplacements(Code, FileAndReplacements.second,
- *Style);
+ format::cleanupAroundReplacements(
+ Code, FileAndReplacements.second.Replacements, *Style);
if (!Replacements) {
llvm::errs() << llvm::toString(Replacements.takeError()) << "\n";
continue;
@@ -226,16 +231,10 @@ class ErrorReporter {
if (!tooling::applyAllReplacements(Replacements.get(), Rewrite)) {
llvm::errs() << "Can't apply replacements for file " << File << "\n";
}
+ AnyNotWritten &= Rewrite.overwriteChangedFiles();
}
- auto BuildDir = Context.getCurrentBuildDirectory();
- if (!BuildDir.empty())
- Rewrite.getSourceMgr()
- .getFileManager()
- .getVirtualFileSystem()
- .setCurrentWorkingDirectory(BuildDir);
-
- if (Rewrite.overwriteChangedFiles()) {
+ if (AnyNotWritten) {
llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
} else {
llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
@@ -297,13 +296,18 @@ class ErrorReporter {
return CharSourceRange::getCharRange(BeginLoc, EndLoc);
}
+ struct ReplacementsWithBuildDir {
+ StringRef BuildDir;
+ Replacements Replacements;
+ };
+
FileManager Files;
LangOptions LangOpts; // FIXME: use langopts from each original file
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
DiagnosticConsumer *DiagPrinter;
DiagnosticsEngine Diags;
SourceManager SourceMgr;
- llvm::StringMap<Replacements> FileReplacements;
+ llvm::StringMap<ReplacementsWithBuildDir> FileReplacements;
ClangTidyContext &Context;
FixBehaviour ApplyFixes;
unsigned TotalFixes = 0U;
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json
index 74275d953727c..53ee3fa7a98da 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json
@@ -20,10 +20,15 @@
"file": "test_dir/b/c.cpp"
},
{
- "directory": "test_dir/b",
- "command": "clang++ -I../include -o test.o ../b/d.cpp",
+ "directory": "test_dir/",
+ "command": "clang++ -o test.o ./b/d.cpp",
"file": "test_dir/b/d.cpp"
},
+{
+ "directory": "test_dir/b",
+ "command": "clang++ -I../include -o test.o ../include.cpp",
+ "file": "test_dir/include.cpp"
+},
{
"directory": "test_dir/",
"command": "clang++ -o test.o test_dir/b/not-exist.cpp",
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp
index f8c05a3f69e95..3c4e8494a4ae3 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp
@@ -5,8 +5,9 @@
// RUN: echo 'int *AB = 0;' > %T/compilation-database-test/a/b.cpp
// RUN: echo 'int *BB = 0;' > %T/compilation-database-test/b/b.cpp
// RUN: echo 'int *BC = 0;' > %T/compilation-database-test/b/c.cpp
+// RUN: echo 'int *BD = 0;' > %T/compilation-database-test/b/d.cpp
// RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h
-// RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp
+// RUN: echo '#include "header.h"' > %T/compilation-database-test/include.cpp
// RUN: sed 's|test_dir|%/T/compilation-database-test|g' %S/Inputs/compilation-database/template.json > %T/compile_commands.json
// Regression test: shouldn't crash.
@@ -15,15 +16,17 @@
// CHECK-NOT-EXIST: unable to handle compilation
// CHECK-NOT-EXIST: Found compiler error
-// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp %T/compilation-database-test/b/d.cpp -header-filter=.* -fix
+// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp %T/compilation-database-test/b/d.cpp %T/compilation-database-test/include.cpp -header-filter=.* -fix
// RUN: FileCheck -input-file=%T/compilation-database-test/a/a.cpp %s -check-prefix=CHECK-FIX1
// RUN: FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s -check-prefix=CHECK-FIX2
// RUN: FileCheck -input-file=%T/compilation-database-test/b/b.cpp %s -check-prefix=CHECK-FIX3
// RUN: FileCheck -input-file=%T/compilation-database-test/b/c.cpp %s -check-prefix=CHECK-FIX4
-// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h %s -check-prefix=CHECK-FIX5
+// RUN: FileCheck -input-file=%T/compilation-database-test/b/d.cpp %s -check-prefix=CHECK-FIX5
+// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h %s -check-prefix=CHECK-FIX6
// CHECK-FIX1: int *AA = nullptr;
// CHECK-FIX2: int *AB = nullptr;
// CHECK-FIX3: int *BB = nullptr;
// CHECK-FIX4: int *BC = nullptr;
-// CHECK-FIX5: int *HP = nullptr;
+// CHECK-FIX5: int *BD = nullptr;
+// CHECK-FIX6: int *HP = nullptr;
More information about the cfe-commits
mailing list