-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Exclude RedirectingFileSystem with null OverlayFileDir in VFSUsage #128267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
An assertion failure like the above was encountered in the swift toolchain in the context of this PR swiftlang/swift#78184 which uses a module map VFS overlay on top of the read-only Windows MSVC/ucrt include paths. |
@@ -149,11 +149,16 @@ std::vector<bool> HeaderSearch::collectVFSUsageAndClear() const { | |||
|
|||
llvm::vfs::FileSystem &RootFS = FileMgr.getVirtualFileSystem(); | |||
// TODO: This only works if the `RedirectingFileSystem`s were all created by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This PR partially addresses the limitation described by this comment.
@llvm/pr-subscribers-clang Author: Hiroshi Yamauchi (hjyamauchi) ChangesThis is to avoid assertion failures like the following when RedirectingFileSystem's are created and used outside createVFSFromOverlayFiles.
Full diff: https://github.com/llvm/llvm-project/pull/128267.diff 1 Files Affected:
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index bf8fe44e4ca9c..f1e011627d499 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -149,11 +149,16 @@ std::vector<bool> HeaderSearch::collectVFSUsageAndClear() const {
llvm::vfs::FileSystem &RootFS = FileMgr.getVirtualFileSystem();
// TODO: This only works if the `RedirectingFileSystem`s were all created by
- // `createVFSFromOverlayFiles`.
+ // `createVFSFromOverlayFiles`. But at least exclude the ones with null
+ // OverlayFileDir.
RootFS.visit([&](llvm::vfs::FileSystem &FS) {
if (auto *RFS = dyn_cast<llvm::vfs::RedirectingFileSystem>(&FS)) {
- VFSUsage.push_back(RFS->hasBeenUsed());
- RFS->clearHasBeenUsed();
+ // Skip a `RedirectingFileSystem` with null OverlayFileDir which indicates
+ // that they aren't created by `createVFSFromOverlayFiles`.
+ if (!RFS->getOverlayFileDir().empty()) {
+ VFSUsage.push_back(RFS->hasBeenUsed());
+ RFS->clearHasBeenUsed();
+ }
}
});
assert(VFSUsage.size() == getHeaderSearchOpts().VFSOverlayFiles.size() &&
|
CC: @Bigcheese |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think this is fine, although it would be nice if there was a tighter connection between the -ivfsoverlay
options and this. I don't think that needs to happen here though.
This is to avoid assertion failures like the following when RedirectingFileSystem's are created and used outside createVFSFromOverlayFiles. ``` Assertion failed: VFSUsage.size() == getHeaderSearchOpts().VFSOverlayFiles.size() && "A different number of RedirectingFileSystem's were present than " "-ivfsoverlay options passed to Clang!", file S:\SourceCache\llvm-project\clang\lib\Lex\HeaderSearch.cpp, line 162 ```
This is to avoid assertion failures like the following when RedirectingFileSystem's are created and used outside createVFSFromOverlayFiles. ``` Assertion failed: VFSUsage.size() == getHeaderSearchOpts().VFSOverlayFiles.size() && "A different number of RedirectingFileSystem's were present than " "-ivfsoverlay options passed to Clang!", file S:\SourceCache\llvm-project\clang\lib\Lex\HeaderSearch.cpp, line 162 ``` Cherrypick commit llvm#128267
This is to avoid assertion failures like the following when RedirectingFileSystem's are created and used outside createVFSFromOverlayFiles.