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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D92160: [c... Kale Chen via Phabricator via cfe-commits

Reply via email to