Skip to content

Serialization: Also serialize the filename for internal storage decls with private accessors #23074

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
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
3 changes: 3 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4549,6 +4549,9 @@ class AbstractStorageDecl : public ValueDecl {
/// other modules.
bool exportsPropertyDescriptor() const;

/// True if any of the accessors to the storage is private or fileprivate.
bool hasPrivateAccessor() const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) {
return D->getKind() >= DeclKind::First_AbstractStorageDecl &&
Expand Down
8 changes: 8 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4419,6 +4419,14 @@ void AbstractStorageDecl::overwriteImplInfo(StorageImplInfo implInfo) {
Accessors.getPointer()->overwriteImplInfo(implInfo);
}

bool AbstractStorageDecl::hasPrivateAccessor() const {
for (auto accessor : getAllAccessors()) {
if (hasPrivateOrFilePrivateFormalAccess(accessor))
return true;
}
return false;
}

void AbstractStorageDecl::setAccessors(StorageImplInfo implInfo,
SourceLoc lbraceLoc,
ArrayRef<AccessorDecl *> accessors,
Expand Down
43 changes: 30 additions & 13 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2693,19 +2693,37 @@ void Serializer::writeDecl(const Decl *D) {
}

if (auto *value = dyn_cast<ValueDecl>(D)) {
if (value->getFormalAccess() <= swift::AccessLevel::FilePrivate &&
!value->getDeclContext()->isLocalContext()) {
auto *storage = dyn_cast<AbstractStorageDecl>(value);
auto access = value->getFormalAccess();
// Emit the private descriminator for private decls.
// FIXME: We shouldn't need to encode this for /all/ private decls.
// In theory we can follow the same rules as mangling and only include
// the outermost private context.
bool shouldEmitPrivateDescriminator =
access <= swift::AccessLevel::FilePrivate &&
!value->getDeclContext()->isLocalContext();

// Emit the the filename for private mapping for private decls and
// decls with private accessors if compiled with -enable-private-imports.
bool shouldEmitFilenameForPrivate =
M->arePrivateImportsEnabled() &&
!value->getDeclContext()->isLocalContext() &&
(access <= swift::AccessLevel::FilePrivate ||
(storage &&
storage->getFormalAccess() >= swift::AccessLevel::Internal &&
storage->hasPrivateAccessor()));

if (shouldEmitFilenameForPrivate || shouldEmitPrivateDescriminator) {
auto topLevelContext = value->getDeclContext()->getModuleScopeContext();
if (auto *enclosingFile = dyn_cast<FileUnit>(topLevelContext)) {
// FIXME: We shouldn't need to encode this for /all/ private decls.
// In theory we can follow the same rules as mangling and only include
// the outermost private context.
Identifier discriminator =
enclosingFile->getDiscriminatorForPrivateValue(value);
unsigned abbrCode =
DeclTypeAbbrCodes[PrivateDiscriminatorLayout::Code];
PrivateDiscriminatorLayout::emitRecord(Out, ScratchRecord, abbrCode,
addDeclBaseNameRef(discriminator));
if (shouldEmitPrivateDescriminator) {
Identifier discriminator =
enclosingFile->getDiscriminatorForPrivateValue(value);
unsigned abbrCode =
DeclTypeAbbrCodes[PrivateDiscriminatorLayout::Code];
PrivateDiscriminatorLayout::emitRecord(
Out, ScratchRecord, abbrCode, addDeclBaseNameRef(discriminator));
}
auto getFilename = [](FileUnit *enclosingFile,
const ValueDecl *decl) -> StringRef {
if (auto *SF = dyn_cast<SourceFile>(enclosingFile)) {
Expand All @@ -2715,8 +2733,7 @@ void Serializer::writeDecl(const Decl *D) {
}
return StringRef();
};
// Only if compiled with -enable-private-imports.
if (M->arePrivateImportsEnabled()) {
if (shouldEmitFilenameForPrivate) {
auto filename = getFilename(enclosingFile, value);
if (!filename.empty()) {
auto filenameID = addFilename(filename);
Expand Down
7 changes: 7 additions & 0 deletions test/Serialization/Inputs/private_import_other.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,10 @@ struct Value {
extension Value {
fileprivate func foo() {}
}

struct Internal {
private(set) var internalVarWithPrivateSetter : Int = 0
fileprivate(set) var internalVarWithFilePrivateSetter : Int = 0
public private(set) var publicVarWithPrivateSetter : Int = 0
public fileprivate(set) var publicVarWithFilePrivateSetter : Int = 0
}
9 changes: 9 additions & 0 deletions test/Serialization/private_import.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@
@_private(sourceFile: "private_import_other.swift") import private_import
@_private(sourceFile: "private_import.swift") import client

extension Internal {
mutating func set() {
self.internalVarWithPrivateSetter = 1
self.internalVarWithFilePrivateSetter = 1
self.publicVarWithPrivateSetter = 1
self.publicVarWithFilePrivateSetter = 1
}
}

Base().foo()
// This should not be ambigious.
Base().bar()
Expand Down