Skip to content

Revert removal of -package-name from swiftinterfaces #77408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2913,14 +2913,6 @@ class ValueDecl : public Decl {
/// \c \@usableFromInline, \c \@inlinalbe, and \c \@_alwaysEmitIntoClient
bool isUsableFromInline() const;

/// Treat as public and allow skipping access checks if the following conditions
/// are met:
/// - This decl has a package access level,
/// - Has a @usableFromInline (or other inlinable) attribute,
/// - And is defined in a module built from a public or private
/// interface that does not contain package-name.
bool isInterfacePackageEffectivelyPublic() const;

/// Returns \c true if this declaration is *not* intended to be used directly
/// by application developers despite the visibility.
bool shouldHideFromEditor() const;
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Frontend/ModuleInterfaceSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ struct ModuleInterfaceOptions {
/// Print imports that are missing from the source and used in API.
bool PrintMissingImports = true;

/// If true, package-name flag is not printed in either public or private
/// interface file.
bool DisablePackageNameForNonPackageInterface = false;

/// Intentionally print invalid syntax into the file.
bool DebugPrintInvalidSyntax = false;

Expand Down
8 changes: 2 additions & 6 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -760,12 +760,8 @@ def disable_bridging_pch : Flag<["-"], "disable-bridging-pch">,

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

def lto : Joined<["-"], "lto=">,
Flags<[FrontendOption, NoInteractiveOption]>,
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1016,8 +1016,7 @@ class Verifier : public ASTWalker {
PrettyStackTraceDecl debugStack("verifying access", D);
if (!D->getASTContext().isAccessControlDisabled()) {
if (D->getFormalAccessScope().isPublic() &&
D->getFormalAccess() < AccessLevel::Public &&
!D->isInterfacePackageEffectivelyPublic()) {
D->getFormalAccess() < AccessLevel::Public) {
Out << "non-public decl has no formal access scope\n";
D->dump(Out);
abort();
Expand Down
38 changes: 3 additions & 35 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4344,30 +4344,6 @@ bool ValueDecl::isUsableFromInline() const {
return false;
}

bool ValueDecl::isInterfacePackageEffectivelyPublic() const {
// A package decl with @usableFromInline (or other inlinable
// attributes) is essentially public, and can be printed in
// public (or private) interface file without package-name;
// it can be referenced by another module (without package-name)
// importing such interface module.
auto isCandidate = getFormalAccess() == AccessLevel::Package &&
isUsableFromInline() &&
getModuleContext()->getPackageName().empty();
if (!isCandidate)
return false;

// Treat the decl as public (1) if it's contained in an interface
// file, e.g. when running -typecheck-module-from-interface or
// -compile-module-from-interface.
isCandidate = false;
if (auto srcFile = getDeclContext()->getParentSourceFile()) {
isCandidate = srcFile->Kind == SourceFileKind::Interface;
}
// Or (2) if the decl being referenced in a client file is defined
// in an interface module.
return isCandidate || getModuleContext()->isBuiltFromInterface();
}

bool ValueDecl::shouldHideFromEditor() const {
// Hide private stdlib declarations.
if (isPrivateSystemDecl(/*treatNonBuiltinProtocolsAsPublic*/ false) ||
Expand Down Expand Up @@ -4460,9 +4436,6 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
if (useDC && VD->getASTContext().isAccessControlDisabled())
return getMaximallyOpenAccessFor(VD);

if (VD->isInterfacePackageEffectivelyPublic())
return AccessLevel::Public;

if (treatUsableFromInlineAsPublic &&
access < AccessLevel::Public &&
VD->isUsableFromInline()) {
Expand Down Expand Up @@ -4645,11 +4618,9 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
case AccessLevel::Package: {
auto pkg = resultDC->getPackageContext(/*lookupIfNotCurrent*/ true);
if (!pkg) {
if (VD->isInterfacePackageEffectivelyPublic())
return AccessScope::getPublic();

// If reached here, should be treated as internal.
return AccessScope(resultDC->getParentModule());
// Instead of reporting and failing early, return the scope of resultDC to
// allow continuation (should still non-zero exit later if in script mode)
return AccessScope(resultDC);
} else {
return AccessScope(pkg);
}
Expand Down Expand Up @@ -4782,9 +4753,6 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
if (VD->getASTContext().isAccessControlDisabled())
return true;

if (VD->isInterfacePackageEffectivelyPublic())
return true;

auto access = getAccessLevel();
auto *sourceDC = VD->getDeclContext();

Expand Down
5 changes: 3 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
Opts.PrintMissingImports =
!Args.hasArg(OPT_disable_print_missing_imports_in_module_interface);
Opts.ABIComments = Args.hasArg(OPT_abi_comments_in_module_interface);
Opts.DisablePackageNameForNonPackageInterface |= Args.hasArg(OPT_disable_print_package_name_for_non_package_interface);

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

static bool IsPackageInterfaceFlag(const Arg *A, ArgList &Args) {
return A->getOption().matches(options::OPT_package_name) &&
!Args.hasArg(
options::OPT_print_package_name_in_non_package_interface);
Args.hasArg(
options::OPT_disable_print_package_name_for_non_package_interface);
}

static bool IsPrivateInterfaceFlag(const Arg *A, ArgList &Args) {
Expand Down
3 changes: 0 additions & 3 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4312,9 +4312,6 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
requiredAccessScope.requiredAccessForDiagnostics();
auto proto = conformance->getProtocol();
auto protoAccessScope = proto->getFormalAccessScope(DC);
if (proto->isInterfacePackageEffectivelyPublic())
return;

bool protoForcesAccess =
requiredAccessScope.hasEqualDeclContextWith(protoAccessScope);
auto diagKind = protoForcesAccess
Expand Down
13 changes: 4 additions & 9 deletions test/ModuleInterface/load_package_interface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
// RUN: %target-swift-frontend -emit-module %t/Bar.swift -I %t \
// RUN: -module-name Bar -package-name foopkg -package-name barpkg \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -disable-print-package-name-for-non-package-interface \
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
Expand All @@ -32,8 +31,7 @@
// RUN: -package-name barpkg \
// RUN: -Rmodule-loading 2> %t/load-pkg-off.output
// RUN: %FileCheck -check-prefix=CHECK-LOAD-PKG-OFF %s < %t/load-pkg-off.output
// CHECK-LOAD-PKG-OFF: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
// CHECK-LOAD-PKG-OFF: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
// 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

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

/// 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.
/// 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.
// RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -package-name barpkg \
// RUN: -experimental-package-interface-load \
// RUN: -Rmodule-loading 2> %t/load-priv.output
// RUN: %FileCheck -check-prefix=CHECK-LOAD-PRIV %s < %t/load-priv.output
// CHECK-LOAD-PRIV: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
// CHECK-LOAD-PRIV: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
// CHECK-LOAD-PRIV: no such module 'Bar'

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

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

//--- Bar.swift
public class PubKlass {
Expand Down
1 change: 1 addition & 0 deletions test/ModuleInterface/package_interface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public enum PubEnum {
case red, green
}

// CHECK: -package-name barpkg
// CHECK: public enum PubEnum {
// CHECK: case red, green
// CHECK: public static func == (a: Bar.PubEnum, b: Bar.PubEnum) -> Swift.Bool
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

/// By default, package-name is only printed in package interface
// RUN: %target-build-swift -emit-module %t/Bar.swift -I %t \
/// Do not print package-name for public or private interfaces
// RUN: %target-build-swift -emit-module %s -I %t \
// RUN: -module-name Bar -package-name foopkg \
// RUN: -enable-library-evolution -swift-version 6 \
// RUN: -package-name barpkg \
// RUN: -Xfrontend -disable-print-package-name-for-non-package-interface \
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
Expand All @@ -22,9 +22,9 @@
// CHECK-PRIVATE-NOT: pkgVar
// CHECK-PACKAGE-NOT: -package-name foopkg

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

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

/// Verify building modules from non-package interfaces succeeds without the package-name flag.
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify

// RUN: rm -rf %t/Bar.swiftmodule
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify

// RUN: rm -rf %t/Bar.swiftmodule
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify

// RUN: rm -rf %t/Bar.swiftmodule
// RUN: rm -rf %t/Bar.swiftinterface
// RUN: rm -rf %t/Bar.private.swiftinterface
// RUN: rm -rf %t/Bar.package.swiftinterface

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

/// Building modules from non-package interfaces with package-name (default mode) should succeed.
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify

// RUN: rm -rf %t/Bar.swiftmodule
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify

// RUN: rm -rf %t/Bar.swiftmodule
// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify

//--- Bar.swift
public struct PubStruct {
public var pubVar: Int
package var pkgVar: Int
}

public struct PubStruct {}
@_spi(bar) public struct SPIStruct {}

package struct PkgStruct {}

@usableFromInline
package struct UfiPkgStruct {
@usableFromInline
package var ufiPkgVar: PubStruct

package var pkgVar: PubStruct

@usableFromInline
package init(_ x: PubStruct, _ y: PubStruct) {
ufiPkgVar = x
pkgVar = y
}
}


//--- Use.swift
import Bar

func use(_ arg: PubStruct) -> PubStruct {
return UfiPkgStruct(arg, arg).ufiPkgVar
}

//--- ExpectFail.swift
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}}

func use(_ arg: PubStruct) -> PubStruct {
return UfiPkgStruct(arg, arg).ufiPkgVar
}
23 changes: 12 additions & 11 deletions test/Sema/accessibility_package_import_interface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@
// RUN: -module-name Dep -swift-version 5 -I %t \
// RUN: -package-name myPkg \
// RUN: -enable-library-evolution \
// RUN: -disable-print-package-name-for-non-package-interface \
// RUN: -emit-module-path %t/Dep.swiftmodule \
// RUN: -emit-module-interface-path %t/Dep.swiftinterface \
// RUN: -emit-private-module-interface-path %t/Dep.private.swiftinterface

// TEST Dep private interface should contain the package name
// RUN: %target-swift-typecheck-module-from-interface(%t/Dep.private.swiftinterface) -module-name Dep -I %t
// RUN: %FileCheck %s --check-prefix=CHECK-DEP-PRIVATE < %t/Dep.private.swiftinterface
// CHECK-DEP-PRIVATE: -package-name myPkg

// TEST Dep.swiftmodule should contain package name and package symbols
// RUN: llvm-bcanalyzer --dump %t/Dep.swiftmodule | %FileCheck %s --check-prefix=CHECK-DEP-BC
// CHECK-DEP-BC: <MODULE_PACKAGE_NAME {{.*}}> blob data = 'myPkg'
// CHECK-DEP-BC: <MODULE_PACKAGE_NAME abbrevid=6/> blob data = 'myPkg'

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

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

// TEST Lib should error on loading Dep built from interface and accessing package symbols (unless usableFromInline or inlinable)
// RUN: %target-swift-frontend -typecheck %t/Lib.swift -package-name myPkg -I %t -verify
Expand All @@ -55,7 +59,6 @@
// RUN: -module-name LibPass -swift-version 5 -I %t \
// RUN: -package-name myPkg \
// RUN: -enable-library-evolution \
// RUN: -disable-print-package-name-for-non-package-interface \
// RUN: -emit-module-path %t/LibPass.swiftmodule \
// RUN: -emit-module-interface-path %t/LibPass.swiftinterface \
// RUN: -emit-private-module-interface-path %t/LibPass.private.swiftinterface
Expand Down Expand Up @@ -83,11 +86,8 @@
@usableFromInline
package class PackageKlassUFI {
@usableFromInline package init() {}
package var packageVar: String = "pkg"

@usableFromInline package var packageVarUFI: String = "pkgUFI"
// expected-note@-1 * {{'packageVarUFI' declared here}}
// expected-error@-1 * {{'packageVarUFI' declared here}}
package var packageVar: String = "pkg"
}

package func packageFunc() {
Expand All @@ -109,7 +109,7 @@ public func publicFuncInlinable() {
}

//--- Lib.swift
import Dep
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}}

public func libFunc() {
publicFuncInlinable()
Expand All @@ -118,7 +118,8 @@ public func libFunc() {
packageFunc() // expected-error {{cannot find 'packageFunc' in scope}}
let x = PackageKlassUFI()
let y = x.packageVarUFI
print(x, y)
let z = x.packageVar // expected-error {{value of type 'PackageKlassUFI' has no member 'packageVar'}}
print(x, y, z)
}


Expand Down
Loading