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

Mon May 2 04:51:16 PDT 2022

Kale added a comment.
Herald added a subscriber: StephenFan.
Herald added a project: All.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92160