avogelsgesang created this revision.
Herald added a subscriber: carlosgalvezp.
avogelsgesang requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
The fixes from the YAML file can refer to relative paths.
Those relative paths are meant to be resolved relative to the
corresponding `build directory`.
However, `clang-apply-replacements` currently interprets all
paths relative to its own working directory. This causes issues,
e.g., when `clang-apply-replacements` is run from outside of
the original build directory.
This commit adjusts `clang-apply-replacements` to take the build
directory into account when resolving relative file paths.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112647
Files:
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h
clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml
clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp
Index: clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp
@@ -0,0 +1,7 @@
+// RUN: mkdir -p %T/Inputs/relative-paths
+// RUN: mkdir -p %T/Inputs/relative-paths/subdir
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/relative-paths/basic.h > %T/Inputs/relative-paths/basic.h
+// RUN: sed "s#\$(path)#%/T/Inputs/relative-paths#" %S/Inputs/relative-paths/file1.yaml > %T/Inputs/relative-paths/file1.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/relative-paths#" %S/Inputs/relative-paths/file2.yaml > %T/Inputs/relative-paths/file2.yaml
+// RUN: clang-apply-replacements %T/Inputs/relative-paths
+// RUN: FileCheck -input-file=%T/Inputs/relative-paths/basic.h %S/Inputs/relative-paths/basic.h
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml
@@ -0,0 +1,15 @@
+---
+MainSourceFile: source2.cpp
+Diagnostics:
+ - BuildDirectory: $(path)
+ DiagnosticName: test-relative-path
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: ./basic.h
+ FileOffset: 148
+ Replacements:
+ - FilePath: ./basic.h
+ Offset: 298
+ Length: 1
+ ReplacementText: elem
+...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml
@@ -0,0 +1,27 @@
+---
+MainSourceFile: source1.cpp
+Diagnostics:
+ - BuildDirectory: $(path)/subdir/
+ DiagnosticName: test-relative-path
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: ../relative-path.h
+ FileOffset: 242
+ Replacements:
+ - FilePath: ../basic.h
+ Offset: 242
+ Length: 26
+ ReplacementText: 'auto & elem : ints'
+ - FilePath: $(path)/basic.h
+ Offset: 276
+ Length: 22
+ ReplacementText: ''
+ - FilePath: ../basic.h
+ Offset: 298
+ Length: 1
+ ReplacementText: elem
+ - FilePath: ../../relative-paths/basic.h
+ Offset: 148
+ Length: 0
+ ReplacementText: 'override '
+...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h
@@ -0,0 +1,32 @@
+#ifndef BASIC_H
+#define BASIC_H
+
+
+class Parent {
+public:
+ virtual void func() {}
+};
+
+class Derived : public Parent {
+public:
+ virtual void func() {}
+ // CHECK: virtual void func() override {}
+};
+
+extern void ext(int (&)[5], const Parent &);
+
+void func(int t) {
+ int ints[5];
+ for (unsigned i = 0; i < 5; ++i) {
+ int &e = ints[i];
+ e = t;
+ // CHECK: for (auto & elem : ints) {
+ // CHECK-NEXT: elem = t;
+ }
+
+ Derived d;
+
+ ext(ints, d);
+}
+
+#endif // BASIC_H
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -196,8 +196,9 @@
auto RefTL = ParamTL.getAs<ReferenceTypeLoc>();
// Do not replace if it is already a value, skip.
- if (RefTL.isNull())
+ if (RefTL.isNull()) {
continue;
+ }
TypeLoc ValueTL = RefTL.getPointeeLoc();
auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getBeginLoc(),
Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -152,9 +152,15 @@
DiagReplacements;
auto AddToGroup = [&](const tooling::Replacement &R,
- const tooling::TranslationUnitDiagnostics *SourceTU) {
+ const tooling::TranslationUnitDiagnostics *SourceTU,
+ const std::string *buildDir) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
+ auto &WorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+ auto PrevWorkingDir = WorkingDir;
+ if (buildDir) {
+ WorkingDir = *buildDir;
+ }
if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
if (SourceTU) {
auto &Replaces = DiagReplacements[*Entry];
@@ -170,18 +176,19 @@
errs() << "Described file '" << R.getFilePath()
<< "' doesn't exist. Ignoring...\n";
}
+ WorkingDir = PrevWorkingDir;
};
for (const auto &TU : TUs)
for (const tooling::Replacement &R : TU.Replacements)
- AddToGroup(R, nullptr);
+ AddToGroup(R, nullptr, nullptr);
for (const auto &TU : TUDs)
for (const auto &D : TU.Diagnostics)
if (const auto *ChoosenFix = tooling::selectFirstFix(D)) {
for (const auto &Fix : *ChoosenFix)
for (const tooling::Replacement &R : Fix.second)
- AddToGroup(R, &TU);
+ AddToGroup(R, &TU, &D.BuildDirectory);
}
// Sort replacements per file to keep consistent behavior when
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
