Skip to content

Conversation

hjyamauchi
Copy link
Contributor

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 
@hjyamauchi
Copy link
Contributor Author

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.

return VFSUsage;

llvm::vfs::FileSystem &RootFS = FileMgr.getVirtualFileSystem();
// TODO: This only works if the `RedirectingFileSystem`s were all created by
Copy link
Contributor Author

@hjyamauchi hjyamauchi Feb 22, 2025

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.

@hjyamauchi hjyamauchi marked this pull request as ready for review February 22, 2025 01:54
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 22, 2025
@hjyamauchi hjyamauchi requested a review from compnerd February 22, 2025 01:54
@llvmbot
Copy link
Member

llvmbot commented Feb 22, 2025

@llvm/pr-subscribers-clang

Author: Hiroshi Yamauchi (hjyamauchi)

Changes

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 

Full diff: https://github.com/llvm/llvm-project/pull/128267.diff

1 Files Affected:

  • (modified) clang/lib/Lex/HeaderSearch.cpp (+8-3)
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() && 
@compnerd
Copy link
Member

Copy link
Contributor

@Bigcheese Bigcheese left a 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 ```
@hjyamauchi hjyamauchi merged commit f58fde5 into llvm:main Feb 25, 2025
11 checks passed
hjyamauchi added a commit to hjyamauchi/llvm-project that referenced this pull request Feb 25, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

4 participants