Skip to content

[NFC] Add 'package' access modifier to AccessLevel #62652

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 5 commits into from
Jan 20, 2023
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
1 change: 0 additions & 1 deletion include/swift/AST/AccessScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ class AccessScope {
}
if (isPackage())
return AS.isPublic();

// If this is public, it can't be less than access level of AS
// so return false
return false;
Expand Down
6 changes: 6 additions & 0 deletions include/swift/AST/AttrKind.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ enum class AccessLevel : uint8_t {
FilePrivate,
/// Internal access is limited to the current module.
Internal,
/// Package access is not limited, but some capabilities may be
/// restricted outside of the current package containing modules.
/// It's similar to Public in that it's accessible from other modules
/// and subclassable only within the defining module as long as
/// the modules are in the same package.
Package,
/// Public access is not limited, but some capabilities may be
/// restricted outside of the current module.
Public,
Expand Down
5 changes: 3 additions & 2 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -693,13 +693,14 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
NumPathElements : 8
);

SWIFT_INLINE_BITFIELD(ExtensionDecl, Decl, 3+1,
SWIFT_INLINE_BITFIELD(ExtensionDecl, Decl, 4+1,
/// An encoding of the default and maximum access level for this extension.
/// The value 4 corresponds to AccessLevel::Public
///
/// This is encoded as (1 << (maxAccess-1)) | (1 << (defaultAccess-1)),
/// which works because the maximum is always greater than or equal to the
/// default, and 'private' is never used. 0 represents an uncomputed value.
DefaultAndMaxAccessLevel : 3,
DefaultAndMaxAccessLevel : 4,

/// Whether there is are lazily-loaded conformances for this extension.
HasLazyConformances : 1
Expand Down
248 changes: 124 additions & 124 deletions include/swift/AST/DiagnosticsSema.def

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/APIDigester/ModuleAnalyzerNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,7 @@ SDKContext::shouldIgnore(Decl *D, const Decl* Parent) const {
case AccessLevel::Private:
case AccessLevel::FilePrivate:
return true;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to revisit this later. I believe this logic is used by the ABI checker to report changes that would break clients that were built against an old ABI and couldn't use the new ABI at runtime. Package clients should always be version locked, the ABI checker can likely ignore package ABI breaks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is it's more restrictive, so making it more permissive later shouldn't be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know. Will add a note to keep track.

Expand Down
3 changes: 3 additions & 0 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,9 @@ class PrintAST : public ASTVisitor<PrintAST> {
case AccessLevel::Public:
Printer << tok::kw_public;
break;
case AccessLevel::Package:
Printer.printKeyword("package", Options);
break;
case AccessLevel::Open:
Printer.printKeyword("open", Options);
break;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ class Verifier : public ASTWalker {
PrettyStackTraceDecl debugStack("verifying access", D);
if (!D->getASTContext().isAccessControlDisabled() &&
D->getFormalAccessScope().isPublic() &&
D->getFormalAccess() < AccessLevel::Public) {
D->getFormalAccess() <= AccessLevel::Package) {
Out << "non-public decl has no formal access scope\n";
D->dump(Out);
abort();
Expand Down
2 changes: 2 additions & 0 deletions lib/AST/AccessRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ DefaultAndMaxAccessLevelRequest::evaluate(Evaluator &evaluator,
maxAccess = AccessLevel::Public;
} else if (maxScope->isPublic()) {
maxAccess = AccessLevel::Public;
} else if (maxScope->isPackage()) {
maxAccess = AccessLevel::Package;
} else if (isa<ModuleDecl>(maxScope->getDeclContext())) {
maxAccess = AccessLevel::Internal;
} else {
Expand Down
1 change: 1 addition & 0 deletions lib/AST/Attr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ StringRef swift::getAccessLevelSpelling(AccessLevel value) {
case AccessLevel::Private: return "private";
case AccessLevel::FilePrivate: return "fileprivate";
case AccessLevel::Internal: return "internal";
case AccessLevel::Package: return "package";
case AccessLevel::Public: return "public";
case AccessLevel::Open: return "open";
}
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3652,6 +3652,7 @@ AccessLevel ValueDecl::getEffectiveAccess() const {
switch (effectiveAccess) {
case AccessLevel::Open:
break;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Internal:
if (getModuleContext()->isTestingEnabled() ||
Expand Down Expand Up @@ -3774,6 +3775,7 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
: AccessLimitKind::None);
case AccessLevel::Internal:
return AccessScope(resultDC->getParentModule());
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
return AccessScope::getPublic();
Expand Down Expand Up @@ -3923,6 +3925,7 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
auto *useSF = dyn_cast<SourceFile>(useFile);
return useSF && useSF->hasTestableOrPrivateImport(access, sourceModule);
}
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
return true;
Expand Down
1 change: 1 addition & 0 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2762,6 +2762,7 @@ bool SourceFile::hasTestableOrPrivateImport(
auto *module = ofDecl->getModuleContext();
switch (accessLevel) {
case AccessLevel::Internal:
case AccessLevel::Package:
case AccessLevel::Public:
// internal/public access only needs an import marked as @_private. The
// filename does not need to match (and we don't serialize it for such
Expand Down
5 changes: 5 additions & 0 deletions lib/IDE/CodeCompletionResultBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ class CodeCompletionResultBuilder {
case AccessLevel::Internal:
// 'internal' is the default, don't add it.
break;
case AccessLevel::Package:
addChunkWithTextNoCopy(
CodeCompletionString::Chunk::ChunkKind::AccessControlKeyword,
"package ");
break;
case AccessLevel::Public:
addChunkWithTextNoCopy(
CodeCompletionString::Chunk::ChunkKind::AccessControlKeyword,
Expand Down
1 change: 1 addition & 0 deletions lib/IRGen/GenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,7 @@ bool IRGenerator::hasLazyMetadata(TypeDecl *type) {
switch (type->getEffectiveAccess()) {
case AccessLevel::Open:
case AccessLevel::Public:
case AccessLevel::Package:
// We can't remove metadata for externally visible types.
return false;
case AccessLevel::Internal:
Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/SIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ FormalLinkage swift::getDeclLinkage(const ValueDecl *D) {
return FormalLinkage::PublicNonUnique;

switch (D->getEffectiveAccess()) {
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
return FormalLinkage::PublicUnique;
Expand Down
2 changes: 2 additions & 0 deletions lib/SIL/IR/SILDeclRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ SILLinkage SILDeclRef::getDefinitionLinkage() const {
return SILLinkage::Shared;
return SILLinkage::Hidden;

case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
switch (limit) {
Expand Down Expand Up @@ -1516,6 +1517,7 @@ SubclassScope SILDeclRef::getSubclassScope() const {
// SILModule, so we don't need to do anything.
return SubclassScope::NotApplicable;
case AccessLevel::Internal:
case AccessLevel::Package:
case AccessLevel::Public:
// If the class is internal or public, it can only be subclassed from
// the same AST Module, but possibly a different SILModule.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole function is interesting for what we discussed about packageopen.

Expand Down
1 change: 1 addition & 0 deletions lib/SIL/IR/SILModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ SILLinkage swift::getDeclSILLinkage(const ValueDecl *decl) {
case AccessLevel::Internal:
linkage = SILLinkage::Hidden;
break;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
linkage = SILLinkage::Public;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Analysis/BasicCalleeAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ void CalleeCache::computeWitnessMethodCalleesForWitnessTable(
switch (Conf->getProtocol()->getEffectiveAccess()) {
case AccessLevel::Open:
llvm_unreachable("protocols cannot have open access level");
case AccessLevel::Package:
case AccessLevel::Public:
canCallUnknown = true;
break;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/IPO/DeadFunctionElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ class DeadFunctionAndGlobalElimination {
case AccessLevel::Internal:
linkage = SILLinkage::Hidden;
break;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
linkage = SILLinkage::Public;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ static bool canBeChangedExternally(SILGlobalVariable *SILG) {
return false;
case AccessLevel::Internal:
return !SILG->getModule().isWholeModule();
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
return true;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ static bool isDefaultCaseKnown(ClassHierarchyAnalysis *CHA,
case AccessLevel::Open:
return false;
case AccessLevel::Public:
case AccessLevel::Package:
case AccessLevel::Internal:
if (!AI.getModule().isWholeModule())
return false;
Expand Down
2 changes: 2 additions & 0 deletions lib/SILOptimizer/Utils/Devirtualize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ static bool isKnownFinalClass(ClassDecl *cd, SILModule &module,
case AccessLevel::Open:
return false;
case AccessLevel::Public:
case AccessLevel::Package:
case AccessLevel::Internal:
if (!module.isWholeModule())
return false;
Expand All @@ -177,6 +178,7 @@ static bool isKnownFinalClass(ClassDecl *cd, SILModule &module,
case AccessLevel::Open:
return false;
case AccessLevel::Public:
case AccessLevel::Package:
case AccessLevel::Internal:
if (!module.isWholeModule())
return false;
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Utils/InstOptUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,7 @@ bool swift::calleesAreStaticallyKnowable(SILModule &module, ValueDecl *vd) {
case AccessLevel::Open:
return false;
case AccessLevel::Public:
case AccessLevel::Package:
if (isa<ConstructorDecl>(vd)) {
// Constructors are special: a derived class in another module can
// "override" a constructor if its class is "open", although the
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5626,6 +5626,7 @@ void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
case AccessLevel::Private: fixItString = "private "; break;
case AccessLevel::FilePrivate: fixItString = "fileprivate "; break;
case AccessLevel::Internal: fixItString = "internal "; break;
case AccessLevel::Package: fixItString = "package "; break;
case AccessLevel::Public: fixItString = "public "; break;
case AccessLevel::Open: fixItString = "open "; break;
}
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/TypeCheckAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2105,6 +2105,7 @@ static void checkExtensionGenericParamAccess(const ExtensionDecl *ED) {
case AccessLevel::Internal:
desiredAccessScope = AccessScope(ED->getModuleContext());
break;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Open:
break;
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3489,6 +3489,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
case AccessLevel::Open:
requiredAccess = AccessLevel::Public;
break;
case AccessLevel::Package:
case AccessLevel::Public:
case AccessLevel::Internal:
requiredAccess = AccessLevel::Internal;
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6180,6 +6180,7 @@ static void diagnoseUnstableName(ProtocolConformance *conformance,
case AccessLevel::Internal:
case AccessLevel::Open:
case AccessLevel::Public:
case AccessLevel::Package:
break;
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2493,6 +2493,7 @@ static Optional<swift::AccessLevel> getActualAccessLevel(uint8_t raw) {
CASE(Private)
CASE(FilePrivate)
CASE(Internal)
CASE(Package)
CASE(Public)
CASE(Open)
#undef CASE
Expand Down
3 changes: 2 additions & 1 deletion lib/Serialization/ModuleFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t SWIFTMODULE_VERSION_MINOR = 735; // @attached
const uint16_t SWIFTMODULE_VERSION_MINOR = 736; // add package access modifier

/// A standard hash seed used for all string hashes in a serialized module.
///
Expand Down Expand Up @@ -521,6 +521,7 @@ enum class AccessLevel : uint8_t {
Private = 0,
FilePrivate,
Internal,
Package,
Public,
Open,
};
Expand Down
1 change: 1 addition & 0 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2195,6 +2195,7 @@ static uint8_t getRawStableAccessLevel(swift::AccessLevel access) {
CASE(Private)
CASE(FilePrivate)
CASE(Internal)
CASE(Package)
CASE(Public)
CASE(Open)
#undef CASE
Expand Down
3 changes: 3 additions & 0 deletions tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,7 @@ namespace {
static UIdent getAccessLevelUID(AccessLevel Access) {
static UIdent AccessOpen("source.lang.swift.accessibility.open");
static UIdent AccessPublic("source.lang.swift.accessibility.public");
static UIdent AccessPackage("source.lang.swift.accessibility.package");
static UIdent AccessInternal("source.lang.swift.accessibility.internal");
static UIdent AccessFilePrivate("source.lang.swift.accessibility.fileprivate");
static UIdent AccessPrivate("source.lang.swift.accessibility.private");
Expand All @@ -1145,6 +1146,8 @@ static UIdent getAccessLevelUID(AccessLevel Access) {
return AccessFilePrivate;
case AccessLevel::Internal:
return AccessInternal;
case AccessLevel::Package:
return AccessPackage;
case AccessLevel::Public:
return AccessPublic;
case AccessLevel::Open:
Expand Down
6 changes: 6 additions & 0 deletions tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ static UIdent Attr_ObjcNamed("source.decl.attribute.objc.name");
static UIdent Attr_Private("source.decl.attribute.private");
static UIdent Attr_FilePrivate("source.decl.attribute.fileprivate");
static UIdent Attr_Internal("source.decl.attribute.internal");
static UIdent Attr_Package("source.decl.attribute.package");
static UIdent Attr_Public("source.decl.attribute.public");
static UIdent Attr_Open("source.decl.attribute.open");
static UIdent Attr_Setter_Private("source.decl.attribute.setter_access.private");
static UIdent Attr_Setter_FilePrivate("source.decl.attribute.setter_access.fileprivate");
static UIdent Attr_Setter_Internal("source.decl.attribute.setter_access.internal");
static UIdent Attr_Setter_Package("source.decl.attribute.setter_access.package");
static UIdent Attr_Setter_Public("source.decl.attribute.setter_access.public");
static UIdent Attr_Setter_Open("source.decl.attribute.setter_access.open");
static UIdent EffectiveAccess_Public("source.decl.effective_access.public");
Expand Down Expand Up @@ -794,6 +796,8 @@ Optional<UIdent> SwiftLangSupport::getUIDForDeclAttribute(const swift::DeclAttri
return Attr_FilePrivate;
case AccessLevel::Internal:
return Attr_Internal;
case AccessLevel::Package:
return Attr_Package;
case AccessLevel::Public:
return Attr_Public;
case AccessLevel::Open:
Expand All @@ -808,6 +812,8 @@ Optional<UIdent> SwiftLangSupport::getUIDForDeclAttribute(const swift::DeclAttri
return Attr_Setter_FilePrivate;
case AccessLevel::Internal:
return Attr_Setter_Internal;
case AccessLevel::Package:
return Attr_Setter_Package;
case AccessLevel::Public:
return Attr_Setter_Public;
case AccessLevel::Open:
Expand Down
1 change: 1 addition & 0 deletions utils/gyb_syntax_support/AttributeKinds.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,7 @@ def __init__(self, name, swift_name=None):
DeclAttributeAlias('fileprivate', 'AccessControl', swift_name='fileprivateKeyword'),
DeclAttributeAlias('internal', 'AccessControl', swift_name='internalKeyword'),
DeclAttributeAlias('public', 'AccessControl', swift_name='publicKeyword'),
ContextualDeclAttributeAlias('package', 'AccessControl'),
ContextualDeclAttributeAlias('open', 'AccessControl'),
DeclAttribute('__setter_access', 'SetterAccess',
OnVar, OnSubscript,
Expand Down