[clang] 07157db - [clang][tidy] Ensure rewriter has the correct CWD (#67839)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 5 15:35:59 PST 2023
Author: Jan Svoboda
Date: 2023-12-05T15:35:55-08:00
New Revision: 07157db81d4421ced9fcf9a2002255c2a3a80d49
URL: https://github.com/llvm/llvm-project/commit/07157db81d4421ced9fcf9a2002255c2a3a80d49
DIFF: https://github.com/llvm/llvm-project/commit/07157db81d4421ced9fcf9a2002255c2a3a80d49.diff
LOG: [clang][tidy] Ensure rewriter has the correct CWD (#67839)
This patch replaces use of the deprecated `FileEntry::getName()` with
`FileEntryRef::getName()`. This means the code now uses the path that
was used to register file entry in `SourceManager` instead of the
absolute path that happened to be used in the last call to
`FileManager::getFile()` some place else.
This caused some test failures due to the fact that some paths can be
relative and thus rely on the VFS CWD. The CWD can change for each TU,
so when we run `clang-tidy` on a compilation database and try to perform
all the replacements at the end, relative paths won't resolve the same.
This patch takes care to reinstate the correct CWD and make the path
reported by `FileEntryRef` absolute before passing it to
`llvm::writeToOutput()`.
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json
clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp
clang/lib/Rewrite/Rewriter.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 4b1a67b6dd98a..baee1d205813b 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,14 @@ class ErrorReporter {
void finish() {
if (TotalFixes > 0) {
- Rewriter Rewrite(SourceMgr, LangOpts);
+ auto &VFS = Files.getVirtualFileSystem();
+ auto OriginalCWD = VFS.getCurrentWorkingDirectory();
+ bool AnyNotWritten = false;
+
for (const auto &FileAndReplacements : FileReplacements) {
+ Rewriter Rewrite(SourceMgr, LangOpts);
StringRef File = FileAndReplacements.first();
+ VFS.setCurrentWorkingDirectory(FileAndReplacements.second.BuildDir);
llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
SourceMgr.getFileManager().getBufferForFile(File);
if (!Buffer) {
@@ -208,8 +215,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,13 +233,18 @@ class ErrorReporter {
if (!tooling::applyAllReplacements(Replacements.get(), Rewrite)) {
llvm::errs() << "Can't apply replacements for file " << File << "\n";
}
+ AnyNotWritten &= Rewrite.overwriteChangedFiles();
}
- if (Rewrite.overwriteChangedFiles()) {
+
+ if (AnyNotWritten) {
llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
} else {
llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
<< TotalFixes << " suggested fixes.\n";
}
+
+ if (OriginalCWD)
+ VFS.setCurrentWorkingDirectory(*OriginalCWD);
}
}
@@ -289,13 +301,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;
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;
More information about the cfe-commits
mailing list