[Serialization] Support Swift only system module #24620
Conversation
rintaro
force-pushed
the
serialization-swiftonly-system-module
branch
from
4a5ad4c to
fcaf92a
Compare
| @@ -13,6 +13,7 @@ | |||
| key.name: "SwiftOnoneSupport", | |||
| key.filepath: SwiftOnoneSupport.swiftmodule, | |||
| key.hash: <hash>, | |||
| key.is_system: 1, | |||
There was a problem hiding this comment.
SwiftOnoneSupport is now system module.
|
@swift-ci Please test |
|
@swift-ci Please test |
|
Build failed |
| @@ -547,6 +557,10 @@ FileUnit *SerializedModuleLoaderBase::loadAST( | |||
| loadInfo.status = | |||
| loadedModuleFile->associateWithFileContext(fileUnit, diagLocOrInvalid, | |||
| treatAsPartialModule); | |||
| if (auto shadowded = loadedModuleFile->getShadowedModule()) | |||
| if (shadowded->isSystemModule()) | |||
There was a problem hiding this comment.
This seems wrong—whether or not a Swift overlay is a system module isn't really dependent on whether the underlying thing is a system module. Is this needed, given that we treat the resource search paths as system search paths now?
Also, typo: "shadowded".
There was a problem hiding this comment.
Is it possible for users to create their own 'user' overlays of system frameworks ?
There was a problem hiding this comment.
I'm not sure it's really needed. I added it just to keep the current behavior. Otherwise bunch of tests fails.
There was a problem hiding this comment.
Is it possible for users to create their own 'user' overlays of system frameworks ?
Possible, yes; supported, not really. And in that case, calling them "system" would be misleading.
I guess since this is the current behavior we can fix it later. Can you leave a comment, at least?
There was a problem hiding this comment.
Can you leave a comment, at least?
Sure. And I'll fix typo.
|
Build failed |
Previously 'isSystemModule()' returns true only if the module is: - Standard library - Clang module and that is `IsSystem` - Swift overlay for clang `IsSystem` module Now: - Clang module and that is `IsSystem`; or - Swift overlay for clang `IsSystem` module - Swift module found in either of these directories: - Runtime library directoris (including stdlib) - Frameworks in `-Fsystem` directories - Frameworks in `$SDKROOT/System/Library/Frameworks/` (Darwin) - Frameworks in `$SDKROOT/Library/Frameworks/` (Darwin) rdar://problem/50516314
rintaro
force-pushed
the
serialization-swiftonly-system-module
branch
from
fcaf92a to
d3d30ee
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
Hm, Linux failure looks like diff --git a/lib/IndexDataStore/IndexDataStore.cpp b/lib/IndexDataStore/IndexDataStore.cpp
index ec7a346935..6f207b4d95 100644
--- a/lib/IndexDataStore/IndexDataStore.cpp
+++ b/lib/IndexDataStore/IndexDataStore.cpp
@@ -85,7 +85,7 @@ bool IndexDataStoreImpl::foreachUnitName(bool sorted,
}
if (sorted) {
- llvm::array_pod_sort(filenames.begin(), filenames.end());
+ std::sort(filenames.begin(), filenames.end());
for (auto &fname : filenames)
if (!receiver(fname))
return false;https://github.com/apple/swift-clang/blob/cd1c5c90d3d0/lib/IndexDataStore/IndexDataStore.cpp#L88 |
|
apple/swift-clang#312 |
|
Maybe they were StringRefs at one point. |
|
Clang repo change has been merged to |
|
Seems that the tests here may be flakey on Windows. |
|
Yikes. Got any more info for us? |
@compnerd https://ci-external.swift.org/job/swift-PR-windows/28/console |
Previously
ModuleDecl::isSystemModule()returns true only if the module is:IsSystemIsSystemmoduleWith this patch, now:
IsSystem; orIsSystemmodule-Fsystemdirectories$SDKROOT/System/Library/Frameworks/(Darwin)$SDKROOT/Library/Frameworks/(Darwin)rdar://problem/50516314