[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

Tue May 3 21:53:43 PDT 2022

OikawaKirie added a comment.

In D92160#3485400 <https://reviews.llvm.org/D92160#3485400>, @Kale wrote:

> In D92160#2449507 <https://reviews.llvm.org/D92160#2449507>, @dexonsmith wrote:
>
>> But it's possible we don't need this. If it's safe for us to update the tests and make `FileManager::getFileRef` always canonicalize to an absolute path, that would definitely be cleaner. `FileManager::makeAbsolute` can use whatever the FS's CWD is at the time of query... nice and clean.
>
> I've tried to fix this bug through this way but found that "making `FileManager::getFileRef` always canonicalize to an absolute path" was pretty hard, because there are many test cases using states stored in FileManager and assuming that they are all in relative form. Besides, the assumption that "the CWD won't change" indeed is correct by design and works fine for single compiler execution. We might not change that unless for a strong necessity.
>
> So I personally believed that this bug is caused by libtooling's incorrect use of `FileManger`. I plan to fix it by implementing a class like `LibtoolingFileManager`, or `AbsoluteFileManager`, which extends the original `FileManager` and using abs path as key to store the status of seen files, but only used for libtooling.
>
> I will try to upload a patch later to verify the possibility.

Thanks a lot for taking over this bug. Please remember to close this patch if your new patch can solve this problem.
I will forward all the comments from the original author (Hao Zhang) if he has any suggestions for your new patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92160