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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
