[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/PiotrZSL approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
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
Date: Fri, 8 Sep 2023 16:39:10 -0700
Subject: [PATCH 1/3] [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
Date: Mon, 4 Dec 2023 14:48:58 -0800
Subject: [PATCH 2/3] [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> Buffer =
SourceMgr.getFileManager().getBufferForFile(File);
if (!Buffer) {
@@ -208,8 +213,8 @@ class ErrorReporter {
continue;
}
llvm::Expected Replacements =
-format::cleanupAroundReplacements(Code, FileAndReplacements.second,
- *Style);
+format::cleanupAroundReplacements(
+C
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
@@ -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(
PiotrZSL wrote:
maybe would be nice to restore CurrentWorkingDirectory after this function come
to the end.
https://github.com/llvm/llvm-project/pull/67839
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
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
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
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> Buffer =
SourceMgr.getFileManager().getBufferForFile(File);
if (!Buffer) {
@@ -208,8 +213,8 @@ class ErrorReporter {
continue;
}
llvm::Expected Replacements =
-format::cleanupAroundReplacements(Code, FileAndReplacements.second,
- *Style);
+format::cleanupAroundReplacements(
+C
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
jansvoboda11 wrote: Ping @PiotrZSL. https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
jansvoboda11 wrote:
> Clang part looks fine. For a clang-tidy part, is there a way to test this
> part ? What changes because we use now a absolute build directory. I'm
> missing some tests for that part.
So the clang-tidy part is necessary to keep the
`clang-tidy/infrastructure/clang-tidy-run-with-database.cpp` test passing. It
uses compilation database that has entries like this:
```json
{
"directory": "/b",
"command": "clang++ -o test.o ../b/c.cpp",
"file": "/b/c.cpp"
}
```
I think this already provides good enough code coverage for this PR. What do
you think?
What I'd like to get clarified in this PR is which directory to use as the CWD.
Is using `ClangTidyContext::getCurrentBuildDirectory()` okay, or do we need to
look somewhere else?
https://github.com/llvm/llvm-project/pull/67839
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/PiotrZSL approved this pull request. Clang part looks fine. For a clang-tidy part, is there a way to test this part ? What changes because we use now a absolute build directory. I'm missing some tests for that part. https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
