[PATCH] D112647: [clang-apply-replacements] Correctly handle relative paths

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.
Herald added a project: All.


================
Comment at: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:178
     }
+    SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;
   };
----------------
This looks incorrect.

The FileManager is not designed to have the working directory change and it's not sound to change/restore it. This is because it caches the meaning of every relative-path lookup.

Consider:
```
/dir1/file1.h
/dir2/file1.h
/dir2/file2.h
```
if you do:
```
FM.WorkingDir = "/dir1";
FM.getFile("file1.h"); // returns /dir1/file1.h
FM.WorkingDir = "/dir2";
FM.getFile("file1.h"); // returns /dir1/file1.h !!!
FM.getFile("file2.h"); // returns /dir2/file2.h
```

See also the comment at https://reviews.llvm.org/D122549#3412965.

I don't know anything about clang-apply-replacements, but I have a couple of ideas for handling this correctly:
- Change the FileManager so that it's constructed with the working directory set to the build directory (don't save+restore, just make it correct from the start).
- If there are multiple concepts of a "build directory", use a different FileManager for each one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112647/new/

https://reviews.llvm.org/D112647