================
@@ -123,91 +143,88 @@ ModuleManager::addModule(StringRef FileName, ModuleKind
Type,
// contents, but we can't check that.)
ExpectedModTime = 0;
}
- // Note: ExpectedSize and ExpectedModTime will be 0 for MK_ImplicitModule
- // when using an ASTFileSignature.
- if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) {
- ErrorStr = IgnoreModTime ? "module file has a different size than expected"
- : "module file has a different size or "
- "modification time than expected";
- return OutOfDate;
- }
- if (!Entry) {
+ std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr);
+ if (!FileKey) {
ErrorStr = "module file not found";
return Missing;
}
- // The ModuleManager's use of FileEntry nodes as the keys for its map of
- // loaded modules is less than ideal. Uniqueness for FileEntry nodes is
- // maintained by FileManager, which in turn uses inode numbers on hosts
- // that support that. When coupled with the module cache's proclivity for
- // turning over and deleting stale PCMs, this means entries for different
- // module files can wind up reusing the same underlying inode. When this
- // happens, subsequent accesses to the Modules map will disagree on the
- // ModuleFile associated with a given file. In general, it is not sufficient
- // to resolve this conundrum with a type like FileEntryRef that stores the
- // name of the FileEntry node on first access because of path
canonicalization
- // issues. However, the paths constructed for implicit module builds are
- // fully under Clang's control. We *can*, therefore, rely on their structure
- // being consistent across operating systems and across subsequent accesses
- // to the Modules map.
- auto implicitModuleNamesMatch = [](ModuleKind Kind, const ModuleFile *MF,
- FileEntryRef Entry) -> bool {
- if (Kind != MK_ImplicitModule)
- return true;
- return Entry.getName() == MF->FileName;
- };
-
- // Check whether we already loaded this module, before
- if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) {
- if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) {
- // Check the stored signature.
- if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
- return OutOfDate;
-
- Module = ModuleEntry;
- updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc);
- return AlreadyLoaded;
+ ModuleFile *ModuleEntry = lookup(*FileKey);
+ if (ModuleEntry)
+ ModuleEntry->Keys.insert(*FileKey);
+
+ // TODO: Remove this.
----------------
jansvoboda11 wrote:
At the time I thought there are multiple places that may look for the same file
with inconsistent explicit/implicit path, so this seemed necessary to
temporarily support both. But now that I look back, I might've been observing
the fact that some tests used `%t` for both an include path and the module
cache, so the `DirectoryEntry` got cached as non-existing. This made
`ModuleManager` lookups fail with the implicit path, and turning it explicit
and just looking up the full path in `FileManager` would succeed. I've removed
this in
[86ee991](https://github.com/llvm/llvm-project/pull/185765/commits/86ee9919673be5a650f94c13a3f33882b6899b35)
and also removed the storage for multiple keys on `ModuleFile` in
[a109526](https://github.com/llvm/llvm-project/pull/185765/commits/a109526c24ebff990aaeb9153a74da5a7ab0fc4d),
since that was there only to support this hack.
LMK if you're happy with how it looks now.
https://github.com/llvm/llvm-project/pull/185765
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
