[clang-tools-extra] 8188484 - [clang-apply-replacements] Correctly handle relative paths

Yitzhak Mandelbaum via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 11 10:30:23 PST 2021


Author: Adrian Vogelsgesang
Date: 2021-11-11T18:28:43Z
New Revision: 8188484daa4195a2c8b5253765036fa2c6da7263

URL: https://github.com/llvm/llvm-project/commit/8188484daa4195a2c8b5253765036fa2c6da7263
DIFF: https://github.com/llvm/llvm-project/commit/8188484daa4195a2c8b5253765036fa2c6da7263.diff

LOG: [clang-apply-replacements] Correctly handle relative paths

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.

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D112647

Added: 
    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

Modified: 
    clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index 1e5012b9891bf..b13b83cf3f000 100644
--- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -23,6 +23,7 @@
 #include "clang/Tooling/DiagnosticsYaml.h"
 #include "clang/Tooling/ReplacementsYaml.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -152,9 +153,13 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
       DiagReplacements;
 
   auto AddToGroup = [&](const tooling::Replacement &R,
-                        const tooling::TranslationUnitDiagnostics *SourceTU) {
+                        const tooling::TranslationUnitDiagnostics *SourceTU,
+                        const llvm::Optional<std::string> BuildDir) {
     // Use the file manager to deduplicate paths. FileEntries are
     // automatically canonicalized.
+    auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+    if (BuildDir)
+      SM.getFileManager().getFileSystemOpts().WorkingDir = std::move(*BuildDir);
     if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
       if (SourceTU) {
         auto &Replaces = DiagReplacements[*Entry];
@@ -170,18 +175,19 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
       errs() << "Described file '" << R.getFilePath()
              << "' doesn't exist. Ignoring...\n";
     }
+    SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;
   };
 
   for (const auto &TU : TUs)
     for (const tooling::Replacement &R : TU.Replacements)
-      AddToGroup(R, nullptr);
+      AddToGroup(R, 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

diff  --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h
new file mode 100644
index 0000000000000..48509684b7725
--- /dev/null
+++ b/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

diff  --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml
new file mode 100644
index 0000000000000..7219ae8ab00e9
--- /dev/null
+++ b/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 '
+...

diff  --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml
new file mode 100644
index 0000000000000..b93942c269b73
--- /dev/null
+++ b/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
+...

diff  --git a/clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp b/clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp
new file mode 100644
index 0000000000000..92cde84134473
--- /dev/null
+++ b/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


        


More information about the cfe-commits mailing list