From af478fd819f601c3dbe0652695f2e8a58b7ef054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Tue, 19 Sep 2023 15:56:47 -0700 Subject: [PATCH] [Sema] Only complain about the import when it does restrict the access level Fix the note pointing to the import when using a package type in a public declaration or inlinable code. We want a note on the import only when it actually lowers the access level of the imported decl. The note about a `package import` was simply be superfluous, while the same note about a `public import` would trigger an assert later on. --- lib/Sema/ResilienceDiagnostics.cpp | 2 +- lib/Sema/TypeCheckAccess.cpp | 6 ++++ .../access-level-import-diag-priority.swift | 31 +++++++++++++++++-- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index d3bcb2b2647ee..22c3c766d31b8 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -114,7 +114,7 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, ImportAccessLevel problematicImport = D->getImportAccessFrom(DC); if (problematicImport.has_value() && - diagAccessLevel == problematicImport->accessLevel) { + problematicImport->accessLevel < D->getFormalAccess()) { Context.Diags.diagnose(problematicImport->accessLevelLoc, diag::decl_import_via_here, D, problematicImport->accessLevel, diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 644d84254cfb8..f19935af007c2 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -268,6 +268,12 @@ void AccessControlCheckerBase::checkTypeAccessImpl( static_cast(complainRepr)->getBoundDecl(); assert(VD && "findTypeWithScope should return bound TypeReprs only"); complainImport = VD->getImportAccessFrom(useDC); + + // Don't complain about an import that doesn't restrict the access + // level of the decl. This can happen with imported `package` decls. + if (complainImport.has_value() && + complainImport->accessLevel >= VD->getFormalAccess()) + complainImport = llvm::None; } diagnose(problematicAccessScope, complainRepr, downgradeToWarning, diff --git a/test/Sema/access-level-import-diag-priority.swift b/test/Sema/access-level-import-diag-priority.swift index 8f3bbc6b5b999..6dd84b0a2cab4 100644 --- a/test/Sema/access-level-import-diag-priority.swift +++ b/test/Sema/access-level-import-diag-priority.swift @@ -4,8 +4,10 @@ // RUN: split-file --leading-lines %s %t /// Build the libraries. -// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t -// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t +// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t \ +// RUN: -package-name pkg +// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t \ +// RUN: -package-name pkg // RUN: %target-swift-frontend -emit-module %t/InternalLib.swift -o %t // RUN: %target-swift-frontend -emit-module %t/FileprivateLib.swift -o %t // RUN: %target-swift-frontend -emit-module %t/PrivateLib.swift -o %t @@ -14,15 +16,20 @@ // RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \ // RUN: -enable-experimental-feature AccessLevelOnImport -verify \ // RUN: -package-name pkg +// RUN: %target-swift-frontend -typecheck %t/PackageTypeImportedAsPackageClient.swift -I %t \ +// RUN: -enable-experimental-feature AccessLevelOnImport -verify \ +// RUN: -package-name pkg // RUN: %target-swift-frontend -typecheck %t/LocalVsImportClient.swift -I %t \ // RUN: -enable-experimental-feature AccessLevelOnImport -verify \ // RUN: -package-name pkg //--- PublicLib.swift public struct PublicImportType {} +package struct PackageLevelPublicImportedType {} //--- PackageLib.swift public struct PackageImportType {} +package struct PackageLevelPackageImportedType {} //--- InternalLib.swift public struct InternalImportType {} @@ -62,6 +69,26 @@ public func publicFuncUsesPrivateScambled(_ a: PublicImportType, d: FileprivateI var _: PrivateImportType } +//--- PackageTypeImportedAsPackageClient.swift +/// Report errors about using package decls in public but don't note the import +/// as it doesn't affect the access level of the decls. + +public import PublicLib +package import PackageLib + +public func publicFuncUsesPackageLevelPublicImportedType(_ a: PackageLevelPublicImportedType) {} // expected-error {{function cannot be declared public because its parameter uses a package type}} +public func publicFuncUsesPackageLevelPackageImportedType(_ a: PackageLevelPackageImportedType) {} // expected-error {{function cannot be declared public because its parameter uses a package type}} + +@inlinable public func funcInlinableReferenceToPublicImportedType() { + var _: PackageLevelPublicImportedType // expected-error {{struct 'PackageLevelPublicImportedType' is package and cannot be referenced from an '@inlinable' function}} + var _: Array // expected-error {{struct 'PackageLevelPublicImportedType' is package and cannot be referenced from an '@inlinable' function}} +} + +@inlinable public func funcInlinableReferenceToPackageImportedType() { + var _: PackageLevelPackageImportedType // expected-error {{struct 'PackageLevelPackageImportedType' is package and cannot be referenced from an '@inlinable' function}} + var _: Array // expected-error {{struct 'PackageLevelPackageImportedType' is package and cannot be referenced from an '@inlinable' function}} +} + /// Local vs imports //--- LocalVsImportClient.swift public import PublicLib