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