Skip to content

Commit 2f112fb

Browse files
jrose-applejrose-apple
committed
Fix '@_usableFromInline import' inference interaction with '@_exported import'
A module might be visible indirectly, because it's @_exported by one of the modules we import (either in Swift, or with the C module map equivalent). In this case, we have to add it to our direct list of imports so that it can be autolinked. This isn't as clean as the previous version because now there's a /second/ list of imports in a SourceFile, but trying to use the regular list of imports to also track modules used from inlinable functions resulted in name lookup /actually looking into these modules/. The other possibility I considered (suggested by Slava) was to make UsableFromInline imports not participate in lookup at all, but that seemed trickier to get right. More rdar://problem/39338239.
1 parent 65b5d54 commit 2f112fb

File tree

10 files changed

+51
-27
lines changed

10 files changed

+51
-27
lines changed

‎include/swift/AST/Module.h

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
351351
// @_exported only.
352352
Public,
353353

354-
// Not @_exported only. Also includes @_usableFromInline.
354+
// Neither @_usableFromInline nor @_exported.
355355
Private,
356356

357357
// @_usableFromInline and @_exported only.
@@ -770,10 +770,6 @@ class SourceFile final : public FileUnit {
770770
/// This source file has access to testable declarations in the imported
771771
/// module.
772772
Testable = 0x2,
773-
774-
/// Modules that depend on the module containing this source file will
775-
/// autolink this dependency.
776-
UsableFromInline = 0x4,
777773
};
778774

779775
/// \see ImportFlags
@@ -786,7 +782,10 @@ class SourceFile final : public FileUnit {
786782
/// This is the list of modules that are imported by this module.
787783
///
788784
/// This is filled in by the Name Binding phase.
789-
MutableArrayRef<std::pair<ModuleDecl::ImportedModule, ImportOptions>> Imports;
785+
ArrayRef<std::pair<ModuleDecl::ImportedModule, ImportOptions>> Imports;
786+
787+
/// A list of modules where declarations were used in inline functions.
788+
llvm::SetVector<ModuleDecl *> ModulesUsedFromInline;
790789

791790
/// A unique identifier representing this file; used to mark private decls
792791
/// within the file to keep them from conflicting with other files in the
@@ -892,7 +891,10 @@ class SourceFile final : public FileUnit {
892891

893892
bool hasTestableImport(const ModuleDecl *module) const;
894893

895-
void markUsableFromInlineImport(const ModuleDecl *module);
894+
void markUsableFromInline(ModuleDecl *module);
895+
ArrayRef<ModuleDecl *> getUsableFromInlineModules() const {
896+
return ModulesUsedFromInline.getArrayRef();
897+
}
896898

897899
void clearLookupCache();
898900

‎lib/AST/Module.cpp

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -975,14 +975,28 @@ SourceFile::getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &modu
975975
continue;
976976
break;
977977
case ModuleDecl::ImportFilter::ForLinking:
978-
if (!importPair.second.contains(ImportFlags::UsableFromInline) &&
979-
!importPair.second.contains(ImportFlags::Exported))
978+
if (!importPair.second.contains(ImportFlags::Exported))
980979
continue;
981980
break;
982981
}
983982

984983
modules.push_back(importPair.first);
985984
}
985+
986+
switch (filter) {
987+
case ModuleDecl::ImportFilter::Public:
988+
case ModuleDecl::ImportFilter::Private:
989+
break;
990+
991+
case ModuleDecl::ImportFilter::All:
992+
case ModuleDecl::ImportFilter::ForLinking:
993+
// Treat UsableAsInline modules like imports.
994+
// This isn't strictly necessary because we would have already found them
995+
// through one of the modules listed above, but it's better for testing.
996+
for (ModuleDecl *module : getUsableFromInlineModules())
997+
modules.emplace_back(ModuleDecl::AccessPathTy(), module);
998+
break;
999+
}
9861000
}
9871001

9881002
void ModuleDecl::getImportedModulesForLookup(
@@ -1287,12 +1301,10 @@ bool SourceFile::hasTestableImport(const swift::ModuleDecl *module) const {
12871301
});
12881302
}
12891303

1290-
void SourceFile::markUsableFromInlineImport(const ModuleDecl *module) {
1291-
for (auto &Import : Imports) {
1292-
if (Import.first.second == module) {
1293-
Import.second |= ImportFlags::UsableFromInline;
1294-
}
1295-
}
1304+
void SourceFile::markUsableFromInline(ModuleDecl *module) {
1305+
if (module == getParentModule())
1306+
return;
1307+
ModulesUsedFromInline.insert(module);
12961308
}
12971309

12981310
void SourceFile::clearLookupCache() {

‎lib/Sema/NameBinding.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,19 @@ void NameBinder::addImport(
235235
ImportOptions options;
236236
if (ID->isExported())
237237
options |= SourceFile::ImportFlags::Exported;
238-
if (ID->isUsableFromInline())
239-
options |= SourceFile::ImportFlags::UsableFromInline;
240238
if (testableAttr)
241239
options |= SourceFile::ImportFlags::Testable;
242240
imports.push_back({ { ID->getDeclPath(), M }, options });
243241

244242
if (topLevelModule != M)
245243
imports.push_back({ { ID->getDeclPath(), topLevelModule }, options });
246244

245+
if (ID->isUsableFromInline()) {
246+
// This is a bit unfortunate; everywhere else in this function, we're not
247+
// actually modifying the SourceFile yet, just building a list for later.
248+
SF.markUsableFromInline(topLevelModule);
249+
}
250+
247251
if (ID->getImportKind() != ImportKind::Module) {
248252
// If we're importing a specific decl, validate the import kind.
249253
using namespace namelookup;

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2516,9 +2516,7 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
25162516
return true;
25172517

25182518
auto *SF = cast<SourceFile>(DC->getModuleScopeContext());
2519-
auto *M = D->getDeclContext()->getParentModule();
2520-
if (SF->getParentModule() != M)
2521-
SF->markUsableFromInlineImport(M);
2519+
SF->markUsableFromInline(D->getModuleContext());
25222520
}
25232521
}
25242522

‎lib/Serialization/ModuleFile.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,8 +1588,8 @@ void ModuleFile::getImportedModules(
15881588
break;
15891589

15901590
case ModuleDecl::ImportFilter::Private:
1591-
// Skip @_exported imports.
1592-
if (dep.isExported())
1591+
// Skip otherwise-labeled imports.
1592+
if (dep.isExported() || dep.isUsableFromInline())
15931593
continue;
15941594

15951595
break;

‎test/Serialization/Inputs/autolinking_module_inferred.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
@_exported import autolinking_public
22
import autolinking_other // inferred as @_usableFromInline
3+
import autolinking_other2
34
import autolinking_private
45

5-
public func bfunc(x: Int = afunc()) {
6+
public func bfunc(x: Int = afunc(), y: Int = afunc2()) {
67
cfunc()
78
}

‎test/Serialization/Inputs/autolinking_other2.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@_exported import autolinking_other3

‎test/Serialization/Inputs/autolinking_other3.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public func afunc2() -> Int { return 0 }

‎test/Serialization/autolinking-inlinable-inferred.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_public.swift -emit-module-path %t/autolinking_public.swiftmodule -module-link-name autolinking_public -swift-version 4
4-
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other.swift -emit-module-path %t/autolinking_other.swiftmodule -module-link-name autolinking_other -swift-version 4
3+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_public.swift -emit-module-path %t/autolinking_public.swiftmodule -module-link-name autolinking_public -I %t -swift-version 4
4+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other3.swift -emit-module-path %t/autolinking_other3.swiftmodule -module-link-name autolinking_other3 -swift-version 4
5+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other2.swift -emit-module-path %t/autolinking_other2.swiftmodule -module-link-name autolinking_other2 -I %t -swift-version 4
6+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other.swift -emit-module-path %t/autolinking_other.swiftmodule -module-link-name autolinking_other -I %t -swift-version 4
57
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_private.swift -emit-module-path %t/autolinking_private.swiftmodule -module-link-name autolinking_private -I %t -swift-version 4
68
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_module_inferred.swift -emit-module-path %t/autolinking_module_inferred.swiftmodule -module-link-name autolinking_module_inferred -I %t -swift-version 4
79
// RUN: %target-swift-frontend -emit-ir %s -I %t -swift-version 4 | %FileCheck %s
@@ -22,11 +24,12 @@ bfunc()
2224

2325
// Note: we don't autolink autolinking_private even though autolinking_module imports it also.
2426

25-
// CHECK: !llvm.linker.options = !{[[SWIFTCORE:![0-9]+]], [[SWIFTONONESUPPORT:![0-9]+]], [[MODULE:![0-9]+]], [[PUBLIC:![0-9]+]], [[OTHER:![0-9]+]], [[OBJC:![0-9]+]]}
27+
// CHECK: !llvm.linker.options = !{[[SWIFTCORE:![0-9]+]], [[SWIFTONONESUPPORT:![0-9]+]], [[MODULE:![0-9]+]], [[PUBLIC:![0-9]+]], [[OTHER3:![0-9]+]], [[OTHER:![0-9]+]], [[OBJC:![0-9]+]]}
2628

2729
// CHECK: [[SWIFTCORE]] = !{!"-lswiftCore"}
2830
// CHECK: [[SWIFTONONESUPPORT]] = !{!"-lswiftSwiftOnoneSupport"}
2931
// CHECK: [[MODULE]] = !{!"-lautolinking_module_inferred"}
3032
// CHECK: [[PUBLIC]] = !{!"-lautolinking_public"}
33+
// CHECK: [[OTHER3]] = !{!"-lautolinking_other3"}
3134
// CHECK: [[OTHER]] = !{!"-lautolinking_other"}
3235
// CHECK: [[OBJC]] = !{!"-lobjc"}

‎test/Serialization/autolinking-inlinable.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// RUN: %empty-directory(%t)
22

33
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_public.swift -emit-module-path %t/autolinking_public.swiftmodule -module-link-name autolinking_public -swift-version 4
4-
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other.swift -emit-module-path %t/autolinking_other.swiftmodule -module-link-name autolinking_other -swift-version 4
4+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other3.swift -emit-module-path %t/autolinking_other3.swiftmodule -module-link-name autolinking_other3 -swift-version 4
5+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other2.swift -emit-module-path %t/autolinking_other2.swiftmodule -module-link-name autolinking_other2 -I %t -swift-version 4
6+
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_other.swift -emit-module-path %t/autolinking_other.swiftmodule -module-link-name autolinking_other -I %t -swift-version 4
57
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_private.swift -emit-module-path %t/autolinking_private.swiftmodule -module-link-name autolinking_private -I %t -swift-version 4
68
// RUN: %target-swift-frontend -emit-module %S/Inputs/autolinking_module.swift -emit-module-path %t/autolinking_module.swiftmodule -module-link-name autolinking_module -I %t -swift-version 4
79
// RUN: %target-swift-frontend -emit-ir %s -I %t -swift-version 4 | %FileCheck %s

0 commit comments

Comments
 (0)