Skip to content

Commit b345418

Browse files
tshortlitshortli
authored
Merge pull request #77408 from tshortli/revert-package-fixes
Revert removal of `-package-name` from swiftinterfaces
2 parents f7f6cd1 + 0e8c66b commit b345418

16 files changed

+70
-154
lines changed

‎include/swift/AST/Decl.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2913,14 +2913,6 @@ class ValueDecl : public Decl {
29132913
/// \c \@usableFromInline, \c \@inlinalbe, and \c \@_alwaysEmitIntoClient
29142914
bool isUsableFromInline() const;
29152915

2916-
/// Treat as public and allow skipping access checks if the following conditions
2917-
/// are met:
2918-
/// - This decl has a package access level,
2919-
/// - Has a @usableFromInline (or other inlinable) attribute,
2920-
/// - And is defined in a module built from a public or private
2921-
/// interface that does not contain package-name.
2922-
bool isInterfacePackageEffectivelyPublic() const;
2923-
29242916
/// Returns \c true if this declaration is *not* intended to be used directly
29252917
/// by application developers despite the visibility.
29262918
bool shouldHideFromEditor() const;

‎include/swift/Frontend/ModuleInterfaceSupport.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ struct ModuleInterfaceOptions {
7272
/// Print imports that are missing from the source and used in API.
7373
bool PrintMissingImports = true;
7474

75+
/// If true, package-name flag is not printed in either public or private
76+
/// interface file.
77+
bool DisablePackageNameForNonPackageInterface = false;
78+
7579
/// Intentionally print invalid syntax into the file.
7680
bool DebugPrintInvalidSyntax = false;
7781

‎include/swift/Option/Options.td

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -760,12 +760,8 @@ def disable_bridging_pch : Flag<["-"], "disable-bridging-pch">,
760760

761761
def disable_print_package_name_for_non_package_interface :
762762
Flag<["-"], "disable-print-package-name-for-non-package-interface">,
763-
Flags<[FrontendOption, NoDriverOption, HelpHidden]>,
764-
HelpText<"No op; package name is only printed in package interface by default">;
765-
def print_package_name_in_non_package_interface :
766-
Flag<["-"], "print-package-name-in-non-package-interface">,
767-
Flags<[FrontendOption, NoDriverOption, HelpHidden]>,
768-
HelpText<"Print package name in public or private interface">;
763+
Flags<[FrontendOption, NoDriverOption, ModuleInterfaceOption, HelpHidden]>,
764+
HelpText<"Disable adding package name to public or private interface">;
769765

770766
def lto : Joined<["-"], "lto=">,
771767
Flags<[FrontendOption, NoInteractiveOption]>,

‎lib/AST/ASTVerifier.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,8 +1016,7 @@ class Verifier : public ASTWalker {
10161016
PrettyStackTraceDecl debugStack("verifying access", D);
10171017
if (!D->getASTContext().isAccessControlDisabled()) {
10181018
if (D->getFormalAccessScope().isPublic() &&
1019-
D->getFormalAccess() < AccessLevel::Public &&
1020-
!D->isInterfacePackageEffectivelyPublic()) {
1019+
D->getFormalAccess() < AccessLevel::Public) {
10211020
Out << "non-public decl has no formal access scope\n";
10221021
D->dump(Out);
10231022
abort();

‎lib/AST/Decl.cpp

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4344,30 +4344,6 @@ bool ValueDecl::isUsableFromInline() const {
43444344
return false;
43454345
}
43464346

4347-
bool ValueDecl::isInterfacePackageEffectivelyPublic() const {
4348-
// A package decl with @usableFromInline (or other inlinable
4349-
// attributes) is essentially public, and can be printed in
4350-
// public (or private) interface file without package-name;
4351-
// it can be referenced by another module (without package-name)
4352-
// importing such interface module.
4353-
auto isCandidate = getFormalAccess() == AccessLevel::Package &&
4354-
isUsableFromInline() &&
4355-
getModuleContext()->getPackageName().empty();
4356-
if (!isCandidate)
4357-
return false;
4358-
4359-
// Treat the decl as public (1) if it's contained in an interface
4360-
// file, e.g. when running -typecheck-module-from-interface or
4361-
// -compile-module-from-interface.
4362-
isCandidate = false;
4363-
if (auto srcFile = getDeclContext()->getParentSourceFile()) {
4364-
isCandidate = srcFile->Kind == SourceFileKind::Interface;
4365-
}
4366-
// Or (2) if the decl being referenced in a client file is defined
4367-
// in an interface module.
4368-
return isCandidate || getModuleContext()->isBuiltFromInterface();
4369-
}
4370-
43714347
bool ValueDecl::shouldHideFromEditor() const {
43724348
// Hide private stdlib declarations.
43734349
if (isPrivateSystemDecl(/*treatNonBuiltinProtocolsAsPublic*/ false) ||
@@ -4460,9 +4436,6 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
44604436
if (useDC && VD->getASTContext().isAccessControlDisabled())
44614437
return getMaximallyOpenAccessFor(VD);
44624438

4463-
if (VD->isInterfacePackageEffectivelyPublic())
4464-
return AccessLevel::Public;
4465-
44664439
if (treatUsableFromInlineAsPublic &&
44674440
access < AccessLevel::Public &&
44684441
VD->isUsableFromInline()) {
@@ -4645,11 +4618,9 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
46454618
case AccessLevel::Package: {
46464619
auto pkg = resultDC->getPackageContext(/*lookupIfNotCurrent*/ true);
46474620
if (!pkg) {
4648-
if (VD->isInterfacePackageEffectivelyPublic())
4649-
return AccessScope::getPublic();
4650-
4651-
// If reached here, should be treated as internal.
4652-
return AccessScope(resultDC->getParentModule());
4621+
// Instead of reporting and failing early, return the scope of resultDC to
4622+
// allow continuation (should still non-zero exit later if in script mode)
4623+
return AccessScope(resultDC);
46534624
} else {
46544625
return AccessScope(pkg);
46554626
}
@@ -4782,9 +4753,6 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
47824753
if (VD->getASTContext().isAccessControlDisabled())
47834754
return true;
47844755

4785-
if (VD->isInterfacePackageEffectivelyPublic())
4786-
return true;
4787-
47884756
auto access = getAccessLevel();
47894757
auto *sourceDC = VD->getDeclContext();
47904758

‎lib/Frontend/CompilerInvocation.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
524524
Opts.PrintMissingImports =
525525
!Args.hasArg(OPT_disable_print_missing_imports_in_module_interface);
526526
Opts.ABIComments = Args.hasArg(OPT_abi_comments_in_module_interface);
527+
Opts.DisablePackageNameForNonPackageInterface |= Args.hasArg(OPT_disable_print_package_name_for_non_package_interface);
527528

528529
if (const Arg *A = Args.getLastArg(OPT_library_level)) {
529530
StringRef contents = A->getValue();
@@ -555,8 +556,8 @@ static bool ShouldIncludeModuleInterfaceArg(const Arg *A) {
555556

556557
static bool IsPackageInterfaceFlag(const Arg *A, ArgList &Args) {
557558
return A->getOption().matches(options::OPT_package_name) &&
558-
!Args.hasArg(
559-
options::OPT_print_package_name_in_non_package_interface);
559+
Args.hasArg(
560+
options::OPT_disable_print_package_name_for_non_package_interface);
560561
}
561562

562563
static bool IsPrivateInterfaceFlag(const Arg *A, ArgList &Args) {

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4312,9 +4312,6 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
43124312
requiredAccessScope.requiredAccessForDiagnostics();
43134313
auto proto = conformance->getProtocol();
43144314
auto protoAccessScope = proto->getFormalAccessScope(DC);
4315-
if (proto->isInterfacePackageEffectivelyPublic())
4316-
return;
4317-
43184315
bool protoForcesAccess =
43194316
requiredAccessScope.hasEqualDeclContextWith(protoAccessScope);
43204317
auto diagKind = protoForcesAccess

‎test/ModuleInterface/load_package_interface.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
// RUN: %target-swift-frontend -emit-module %t/Bar.swift -I %t \
66
// RUN: -module-name Bar -package-name foopkg -package-name barpkg \
77
// RUN: -enable-library-evolution -swift-version 5 \
8-
// RUN: -disable-print-package-name-for-non-package-interface \
98
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
109
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
1110
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -32,8 +31,7 @@
3231
// RUN: -package-name barpkg \
3332
// RUN: -Rmodule-loading 2> %t/load-pkg-off.output
3433
// RUN: %FileCheck -check-prefix=CHECK-LOAD-PKG-OFF %s < %t/load-pkg-off.output
35-
// CHECK-LOAD-PKG-OFF: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
36-
// CHECK-LOAD-PKG-OFF: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
34+
// CHECK-LOAD-PKG-OFF: error: module 'Bar' is in package 'barpkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface:{{.*}}Bar.private.swiftinterface
3735

3836
/// Client loads a private interface since the package-name is different from the loaded module's.
3937
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
@@ -47,15 +45,13 @@
4745
// RUN: rm -rf %t/*.swiftmodule
4846
// RUN: rm -rf %t/Bar.package.swiftinterface
4947

50-
/// Client loads a private interface since package interface doesn't exist. It should error since the
51-
/// loaded module is not built from a package interface.
48+
/// Client loads a private interface since package interface doesn't exist. It should error since the loaded module is not built from a package interface.
5249
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
5350
// RUN: -package-name barpkg \
5451
// RUN: -experimental-package-interface-load \
5552
// RUN: -Rmodule-loading 2> %t/load-priv.output
5653
// RUN: %FileCheck -check-prefix=CHECK-LOAD-PRIV %s < %t/load-priv.output
57-
// CHECK-LOAD-PRIV: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
58-
// CHECK-LOAD-PRIV: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
54+
// CHECK-LOAD-PRIV: no such module 'Bar'
5955

6056
// RUN: rm -rf %t/*.swiftmodule
6157
// RUN: rm -rf %t/Bar.private.swiftinterface
@@ -68,8 +64,7 @@
6864
// RUN: -Rmodule-loading 2> %t/load-pub.output
6965

7066
// RUN: %FileCheck -check-prefix=CHECK-LOAD-PUB %s < %t/load-pub.output
71-
// CHECK-LOAD-PUB: remark: loaded module 'Bar'; source: '{{.*}}Bar.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
72-
// CHECK-LOAD-PUB: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
67+
// CHECK-LOAD-PUB: no such module 'Bar'
7368

7469
//--- Bar.swift
7570
public class PubKlass {

‎test/ModuleInterface/package_interface.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public enum PubEnum {
2323
case red, green
2424
}
2525

26+
// CHECK: -package-name barpkg
2627
// CHECK: public enum PubEnum {
2728
// CHECK: case red, green
2829
// CHECK: public static func == (a: Bar.PubEnum, b: Bar.PubEnum) -> Swift.Bool

‎test/ModuleInterface/print_package_name_in_non_package_interface.swift renamed to ‎test/ModuleInterface/package_interface_disable_package_name.swift

Lines changed: 9 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: split-file %s %t
32

4-
/// By default, package-name is only printed in package interface
5-
// RUN: %target-build-swift -emit-module %t/Bar.swift -I %t \
3+
/// Do not print package-name for public or private interfaces
4+
// RUN: %target-build-swift -emit-module %s -I %t \
65
// RUN: -module-name Bar -package-name foopkg \
76
// RUN: -enable-library-evolution -swift-version 6 \
87
// RUN: -package-name barpkg \
8+
// RUN: -Xfrontend -disable-print-package-name-for-non-package-interface \
99
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
1010
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
1111
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -22,9 +22,9 @@
2222
// CHECK-PRIVATE-NOT: pkgVar
2323
// CHECK-PACKAGE-NOT: -package-name foopkg
2424

25-
// CHECK-PUBLIC: -enable-library-evolution -swift-version 6 -module-name Bar
26-
// CHECK-PRIVATE: -enable-library-evolution -swift-version 6 -module-name Bar
27-
// CHECK-PACKAGE: -enable-library-evolution -swift-version 6 -module-name Bar -package-name barpkg
25+
// CHECK-PUBLIC: -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
26+
// CHECK-PRIVATE: -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
27+
// CHECK-PACKAGE: -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar -package-name barpkg
2828

2929
/// Typechecking interface files (without package-name in non-package interface) should succeed.
3030
// RUN: %target-swift-frontend -typecheck-module-from-interface %t/Bar.swiftinterface
@@ -33,31 +33,21 @@
3333

3434
/// Verify building modules from non-package interfaces succeeds without the package-name flag.
3535
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
36-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
37-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify
38-
3936
// RUN: rm -rf %t/Bar.swiftmodule
4037
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
41-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
42-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify
43-
4438
// RUN: rm -rf %t/Bar.swiftmodule
4539
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
46-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify
4740

4841
// RUN: rm -rf %t/Bar.swiftmodule
4942
// RUN: rm -rf %t/Bar.swiftinterface
5043
// RUN: rm -rf %t/Bar.private.swiftinterface
5144
// RUN: rm -rf %t/Bar.package.swiftinterface
5245

53-
/// Print -package-name in public or private interface.
54-
/// Note the order of arguments differs across old/new driver, so force old
55-
/// driver for now.
46+
/// By default, -package-name is printed in all interfaces.
5647
// RUN: env SWIFT_USE_OLD_DRIVER=1 %target-build-swift \
57-
// RUN: -emit-module %t/Bar.swift -I %t \
48+
// RUN: -emit-module %s -I %t \
5849
// RUN: -module-name Bar -package-name barpkg \
5950
// RUN: -enable-library-evolution -swift-version 6 \
60-
// RUN: -Xfrontend -print-package-name-in-non-package-interface \
6151
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
6252
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
6353
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -70,51 +60,12 @@
7060

7161
/// Building modules from non-package interfaces with package-name (default mode) should succeed.
7262
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
73-
// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify
74-
7563
// RUN: rm -rf %t/Bar.swiftmodule
7664
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
77-
// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify
78-
7965
// RUN: rm -rf %t/Bar.swiftmodule
8066
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
81-
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify
82-
83-
//--- Bar.swift
84-
public struct PubStruct {
85-
public var pubVar: Int
86-
package var pkgVar: Int
87-
}
8867

68+
public struct PubStruct {}
8969
@_spi(bar) public struct SPIStruct {}
9070

9171
package struct PkgStruct {}
92-
93-
@usableFromInline
94-
package struct UfiPkgStruct {
95-
@usableFromInline
96-
package var ufiPkgVar: PubStruct
97-
98-
package var pkgVar: PubStruct
99-
100-
@usableFromInline
101-
package init(_ x: PubStruct, _ y: PubStruct) {
102-
ufiPkgVar = x
103-
pkgVar = y
104-
}
105-
}
106-
107-
108-
//--- Use.swift
109-
import Bar
110-
111-
func use(_ arg: PubStruct) -> PubStruct {
112-
return UfiPkgStruct(arg, arg).ufiPkgVar
113-
}
114-
115-
//--- ExpectFail.swift
116-
import Bar // expected-error {{module 'Bar' is in package 'barpkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface}}
117-
118-
func use(_ arg: PubStruct) -> PubStruct {
119-
return UfiPkgStruct(arg, arg).ufiPkgVar
120-
}

‎test/Sema/accessibility_package_import_interface.swift

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@
66
// RUN: -module-name Dep -swift-version 5 -I %t \
77
// RUN: -package-name myPkg \
88
// RUN: -enable-library-evolution \
9-
// RUN: -disable-print-package-name-for-non-package-interface \
109
// RUN: -emit-module-path %t/Dep.swiftmodule \
1110
// RUN: -emit-module-interface-path %t/Dep.swiftinterface \
1211
// RUN: -emit-private-module-interface-path %t/Dep.private.swiftinterface
1312

13+
// TEST Dep private interface should contain the package name
14+
// RUN: %target-swift-typecheck-module-from-interface(%t/Dep.private.swiftinterface) -module-name Dep -I %t
15+
// RUN: %FileCheck %s --check-prefix=CHECK-DEP-PRIVATE < %t/Dep.private.swiftinterface
16+
// CHECK-DEP-PRIVATE: -package-name myPkg
17+
1418
// TEST Dep.swiftmodule should contain package name and package symbols
1519
// RUN: llvm-bcanalyzer --dump %t/Dep.swiftmodule | %FileCheck %s --check-prefix=CHECK-DEP-BC
16-
// CHECK-DEP-BC: <MODULE_PACKAGE_NAME {{.*}}> blob data = 'myPkg'
20+
// CHECK-DEP-BC: <MODULE_PACKAGE_NAME abbrevid=6/> blob data = 'myPkg'
1721

1822
// TEST Lib should load Dep.swiftmodule and access package decls if in the same package and error if not
1923
// RUN: %target-swift-frontend -typecheck %t/Lib.swift -package-name myPkg -I %t
@@ -35,9 +39,9 @@
3539
// RUN: -module-name Dep -I %t \
3640
// RUN: -o %t/Dep.swiftmodule
3741

38-
// TEST Dep binary built from interface should not contain package name or package symbols.
42+
// TEST Dep binary built from interface should contain package name but no package symbols
3943
// RUN: llvm-bcanalyzer --dump %t/Dep.swiftmodule | %FileCheck %s --check-prefix=CHECK-DEP-INTER-BC
40-
// CHECK-DEP-INTER-BC-NOT: <MODULE_PACKAGE_NAME {{.*}}> blob data = 'myPkg'
44+
// CHECK-DEP-INTER-BC: <MODULE_PACKAGE_NAME abbrevid=7/> blob data = 'myPkg'
4145

4246
// TEST Lib should error on loading Dep built from interface and accessing package symbols (unless usableFromInline or inlinable)
4347
// RUN: %target-swift-frontend -typecheck %t/Lib.swift -package-name myPkg -I %t -verify
@@ -55,7 +59,6 @@
5559
// RUN: -module-name LibPass -swift-version 5 -I %t \
5660
// RUN: -package-name myPkg \
5761
// RUN: -enable-library-evolution \
58-
// RUN: -disable-print-package-name-for-non-package-interface \
5962
// RUN: -emit-module-path %t/LibPass.swiftmodule \
6063
// RUN: -emit-module-interface-path %t/LibPass.swiftinterface \
6164
// RUN: -emit-private-module-interface-path %t/LibPass.private.swiftinterface
@@ -83,11 +86,8 @@
8386
@usableFromInline
8487
package class PackageKlassUFI {
8588
@usableFromInline package init() {}
86-
package var packageVar: String = "pkg"
87-
8889
@usableFromInline package var packageVarUFI: String = "pkgUFI"
89-
// expected-note@-1 * {{'packageVarUFI' declared here}}
90-
// expected-error@-1 * {{'packageVarUFI' declared here}}
90+
package var packageVar: String = "pkg"
9191
}
9292

9393
package func packageFunc() {
@@ -109,7 +109,7 @@ public func publicFuncInlinable() {
109109
}
110110

111111
//--- Lib.swift
112-
import Dep
112+
import Dep // expected-error {{module 'Dep' is in package 'myPkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface}}
113113

114114
public func libFunc() {
115115
publicFuncInlinable()
@@ -118,7 +118,8 @@ public func libFunc() {
118118
packageFunc() // expected-error {{cannot find 'packageFunc' in scope}}
119119
let x = PackageKlassUFI()
120120
let y = x.packageVarUFI
121-
print(x, y)
121+
let z = x.packageVar // expected-error {{value of type 'PackageKlassUFI' has no member 'packageVar'}}
122+
print(x, y, z)
122123
}
123124

124125

0 commit comments

Comments
 (0)