User Details
- User Since
- Oct 7 2020, 6:11 PM (95 w, 4 d)
Fri, Aug 5
Wed, Aug 3
Tue, Aug 2
Thanks 🙇
Mon, Aug 1
LGTM, thanks for doing this 🙇
Jun 13 2022
The failure here is likely due to the hack in FileManager::getFileRef:
// FIXME: This hack ensures that `getDir()` will use the path that was // used to lookup this file, even if we found a file by different path // first. This is required in order to find a module's structure when its // headers/module map are mapped in the VFS.
So the directory returned from the FileEntry is the *lookup* directory, but the FileEntryRef will be giving back the directory from the external path (if it has been remapped).
Failing test seems to be because . is turned into the full path at some point. It's possible that this is because getFileRef returns the absolute path (see the massive FIXME there). If that's the case we could fix that by fixing clang-apply-replacements and then change the Status.getName() == Filename to only check whether it's an externally mapped path.
Few small comments, LGTM otherwise.
May 9 2022
May 6 2022
Apr 22 2022
Remove line for <unknown> check
Update title/message
After speaking with Ben, we decided it makes more sense to just remove the reference entirely.
Apr 15 2022
Shadowing of FE almost tripped me up there 😅
Apr 14 2022
Thanks for doing all these Jan!
Apr 12 2022
Thanks Jan :)!
Apr 11 2022
Apr 8 2022
Does clang/test/VFS/external-names-multi-overlay.c need to be formatted or is it fine? It uses split-file so I'd really like to avoid the format here (makes it pretty silly).
Looks like there's more changes required for modulemap-collision.m to actually pass. I'll try figure those out when I have the time.
Apr 5 2022
Added a potential plan to remove the FileManager hacks.
Apr 4 2022
This broke crash reproducers in very specific circumstances, see https://reviews.llvm.org/D123104. I'll re-commit with adding ExposesExternalVFSPath instead of replacing IsVFSMapped.
Mar 30 2022
Mar 29 2022
Keep using the redirection hack for the time being
Mar 28 2022
Rename IsVFSMapped as suggested by Duncan. Update comments.
clang-apply-replacements/relative-paths.cpp is failing, I haven't looked into it but my guess would be that it's from the Status.getName() == Filename -> !Status.IsVFSMapped change. That seems very odd to me.
Mar 27 2022
Mar 18 2022
Mar 17 2022
Blocked on the dependents
Blocked on the dependent
We had some discussions around the issue of needing to replace the returned path with the original, but that not being possible because of use-external-names: true. I'm going to put up a separate patch that should make it possible to detect when an external path was returned and then not replace in that case. We can then use that here.
Mar 16 2022
Remove RedirectingFileSystem::dump (got re-added in the rebase)
Slight description update
Mar 15 2022
There's two failing tests with this change:
- VFSFromYAMLTest.ReturnsExternalPathVFSHit
- VFSFromYAMLTest.ReturnsInternalPathVFSHit
Merged into D121425 instead.
Re-order to be after D121425
Pass absolute paths down into contained FS
Final review comments and small addition
Mar 14 2022
Forgot to update dump (have a release build locally)
Address review comments
Mar 11 2022
Removed LLVM_DUMP_METHOD from .cpp
Added VFS.cpp, removed implementation from header
@dexonsmith added you to reviewers since you were automatically added as a subscriber :)