vsapsai created this revision.
vsapsai added reviewers: bnbarham, ChuanqiXu.
Herald added a subscriber: ributzka.
Herald added a project: All.
vsapsai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When a framework can be found at a new location, all references to it in
the module cache become outdated. When we try to load such outdated .pcm
file, we shouldn't change any already loaded and processed modules.
If `Module` has `ASTFile`, it means we've read its AST block already and
it is too late to undo that. If `ASTFile` is `None`, there is no value
in setting it to `None` again. So we don't reset `ASTFile` in
`ModuleManager::removeModules` at all.
rdar://97216258
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134249
Files:
clang/include/clang/Basic/Module.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ModuleManager.cpp
clang/test/Modules/dependent-module-different-location.m
Index: clang/test/Modules/dependent-module-different-location.m
===================================================================
--- /dev/null
+++ clang/test/Modules/dependent-module-different-location.m
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+//
+// At first build Stable.pcm that references Movable.framework from StableFrameworks.
+// RUN: %clang_cc1 -fsyntax-only -F %t/JustBuilt -F %t/StableFrameworks %t/prepopulate-module-cache.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+//
+// Now add Movable.framework to JustBuilt.
+// RUN: mkdir %t/JustBuilt
+// RUN: cp -r %t/StableFrameworks/Movable.framework %t/JustBuilt/Movable.framework
+//
+// Load Movable.pcm at first for JustBuilt location and then in the same TU try to load transitively for StableFrameworks location.
+// RUN: %clang_cc1 -fsyntax-only -F %t/JustBuilt -F %t/StableFrameworks %t/trigger-error.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test the case when a dependent module is found in a different location, so
+// module cache has outdated information. <rdar://97216258>
+
+//--- StableFrameworks/Movable.framework/Headers/Movable.h
+// empty
+
+//--- StableFrameworks/Movable.framework/Modules/module.modulemap
+framework module Movable {
+ header "Movable.h"
+ export *
+}
+
+
+//--- StableFrameworks/Stable.framework/Headers/Stable.h
+#import <Movable/Movable.h>
+
+//--- StableFrameworks/Stable.framework/Modules/module.modulemap
+framework module Stable {
+ header "Stable.h"
+ export *
+}
+
+
+//--- prepopulate-module-cache.m
+#import <Stable/Stable.h>
+
+//--- trigger-error.m
+#import <Movable/Movable.h>
+#import <Stable/Stable.h>
Index: clang/lib/Serialization/ModuleManager.cpp
===================================================================
--- clang/lib/Serialization/ModuleManager.cpp
+++ clang/lib/Serialization/ModuleManager.cpp
@@ -249,7 +249,7 @@
return NewlyLoaded;
}
-void ModuleManager::removeModules(ModuleIterator First, ModuleMap *modMap) {
+void ModuleManager::removeModules(ModuleIterator First) {
auto Last = end();
if (First == Last)
return;
@@ -280,19 +280,11 @@
}
}
- // Delete the modules and erase them from the various structures.
+ // Delete the modules.
for (ModuleIterator victim = First; victim != Last; ++victim) {
Modules.erase(victim->File);
-
- if (modMap) {
- StringRef ModuleName = victim->ModuleName;
- if (Module *mod = modMap->findModule(ModuleName)) {
- mod->setASTFile(None);
- }
- }
}
- // Delete the modules.
Chain.erase(Chain.begin() + (First - begin()), Chain.end());
}
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4227,10 +4227,7 @@
ReadASTCore(FileName, Type, ImportLoc,
/*ImportedBy=*/nullptr, Loaded, 0, 0, ASTFileSignature(),
ClientLoadCapabilities)) {
- ModuleMgr.removeModules(ModuleMgr.begin() + NumModules,
- PP.getLangOpts().Modules
- ? &PP.getHeaderSearchInfo().getModuleMap()
- : nullptr);
+ ModuleMgr.removeModules(ModuleMgr.begin() + NumModules);
// If we find that any modules are unusable, the global index is going
// to be out-of-date. Just remove it.
Index: clang/include/clang/Serialization/ModuleManager.h
===================================================================
--- clang/include/clang/Serialization/ModuleManager.h
+++ clang/include/clang/Serialization/ModuleManager.h
@@ -250,7 +250,7 @@
std::string &ErrorStr);
/// Remove the modules starting from First (to the end).
- void removeModules(ModuleIterator First, ModuleMap *modMap);
+ void removeModules(ModuleIterator First);
/// Add an in-memory buffer the list of known buffers
void addInMemoryBuffer(StringRef FileName,
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -608,8 +608,7 @@
/// Set the serialized AST file for the top-level module of this module.
void setASTFile(Optional<FileEntryRef> File) {
- assert((!File || !getASTFile() || getASTFile() == File) &&
- "file path changed");
+ assert((!getASTFile() || getASTFile() == File) && "file path changed");
getTopLevelModule()->ASTFile = File;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
