Skip to content

Use 'fileprivate' for synthesized members of 'private' decls #15980

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 1 commit into from
Apr 17, 2018
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
10 changes: 8 additions & 2 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2304,8 +2304,14 @@ class ValueDecl : public Decl {
bool treatUsableFromInlineAsPublic = false) const;


/// Copy the formal access level and @usableFromInline attribute from source.
void copyFormalAccessFrom(ValueDecl *source);
/// Copy the formal access level and @usableFromInline attribute from
/// \p source.
///
/// If \p sourceIsParentContext is true, an access level of \c private will
/// be copied as \c fileprivate, to ensure that this declaration will be
/// available everywhere \p source is.
void copyFormalAccessFrom(const ValueDecl *source,
bool sourceIsParentContext = false);

/// Returns the access level that actually controls how a declaration should
/// be emitted and may be used.
Expand Down
21 changes: 18 additions & 3 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2259,9 +2259,24 @@ ValueDecl::getFormalAccessScope(const DeclContext *useDC,
llvm_unreachable("unknown access level");
}

void ValueDecl::copyFormalAccessFrom(ValueDecl *source) {
void ValueDecl::copyFormalAccessFrom(const ValueDecl *source,
bool sourceIsParentContext) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need sourceIsParentContext, or can you just check if the source's parent DeclContext is a SourceFile, and if so, turn Private to FilePrivate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's relevant for any type, unfortunately, not just those at the top level. If we wanted to do it without the flag we could check if source and this are related in some way, but that seems more complicated for something every call site already knows.

if (!hasAccess()) {
setAccess(source->getFormalAccess());
AccessLevel access = source->getFormalAccess();

// To make something have the same access as a 'private' parent, it has to
// be 'fileprivate' or greater.
if (sourceIsParentContext && access == AccessLevel::Private)
access = AccessLevel::FilePrivate;

// Only certain declarations can be 'open'.
if (access == AccessLevel::Open && !isPotentiallyOverridable()) {
assert(!isa<ClassDecl>(this) &&
"copying 'open' onto a class has complications");
access = AccessLevel::Public;
}

setAccess(access);
}

// Inherit the @usableFromInline attribute.
Expand Down Expand Up @@ -2816,7 +2831,7 @@ void ClassDecl::addImplicitDestructor() {
setHasDestructor();

// Propagate access control and versioned-ness.
DD->copyFormalAccessFrom(this);
DD->copyFormalAccessFrom(this, /*sourceIsParentContext*/true);

// Wire up generic environment of DD.
DD->setGenericEnvironment(getGenericEnvironmentOfContext());
Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/DerivedConformanceCodable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ static FuncDecl *deriveEncodable_encode(TypeChecker &tc, Decl *parentDecl,

encodeDecl->setInterfaceType(interfaceType);
encodeDecl->setValidationStarted();
encodeDecl->setAccess(target->getFormalAccess());
encodeDecl->copyFormalAccessFrom(target, /*sourceIsParentContext*/true);

tc.Context.addSynthesizedDecl(encodeDecl);

Expand Down Expand Up @@ -1106,7 +1106,7 @@ static ValueDecl *deriveDecodable_init(TypeChecker &tc, Decl *parentDecl,
initDecl->setInterfaceType(interfaceType);
initDecl->setValidationStarted();
initDecl->setInitializerInterfaceType(initializerType);
initDecl->setAccess(target->getFormalAccess());
initDecl->copyFormalAccessFrom(target, /*sourceIsParentContext*/true);

tc.Context.addSynthesizedDecl(initDecl);

Expand Down
6 changes: 3 additions & 3 deletions lib/Sema/DerivedConformanceEquatableHashable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ deriveEquatable_eq(TypeChecker &tc, Decl *parentDecl, NominalTypeDecl *typeDecl,
FunctionType::ExtInfo());
}
eqDecl->setInterfaceType(interfaceTy);
eqDecl->copyFormalAccessFrom(typeDecl);
eqDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
eqDecl->setValidationStarted();

tc.Context.addSynthesizedDecl(eqDecl);
Expand Down Expand Up @@ -1044,15 +1044,15 @@ deriveHashable_hashValue(TypeChecker &tc, Decl *parentDecl,

getterDecl->setInterfaceType(interfaceType);
getterDecl->setValidationStarted();
getterDecl->copyFormalAccessFrom(typeDecl);
getterDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);

// Finish creating the property.
hashValueDecl->setImplicit();
hashValueDecl->setInterfaceType(intType);
hashValueDecl->setValidationStarted();
hashValueDecl->makeComputed(SourceLoc(), getterDecl,
nullptr, nullptr, SourceLoc());
hashValueDecl->copyFormalAccessFrom(typeDecl);
hashValueDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);

Pattern *hashValuePat = new (C) NamedPattern(hashValueDecl, /*implicit*/true);
hashValuePat->setType(intType);
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/DerivedConformanceRawRepresentable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ static ConstructorDecl *deriveRawRepresentable_init(TypeChecker &tc,
}
initDecl->setInterfaceType(allocIfaceType);
initDecl->setInitializerInterfaceType(initIfaceType);
initDecl->copyFormalAccessFrom(enumDecl);
initDecl->copyFormalAccessFrom(enumDecl, /*sourceIsParentContext*/true);
initDecl->setValidationStarted();

tc.Context.addSynthesizedDecl(initDecl);
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/DerivedConformances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ DerivedConformance::declareDerivedProperty(TypeChecker &tc, Decl *parentDecl,
/*IsCaptureList*/false, SourceLoc(), name,
propertyContextType, parentDC);
propDecl->setImplicit();
propDecl->copyFormalAccessFrom(typeDecl);
propDecl->copyFormalAccessFrom(typeDecl, /*sourceIsParentContext*/true);
propDecl->setInterfaceType(propertyInterfaceType);
propDecl->setValidationStarted();

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7605,7 +7605,7 @@ void TypeChecker::validateDecl(ValueDecl *D) {
&& "Decl parsing must prevent destructors outside of types!");

checkDeclAttributesEarly(DD);
DD->copyFormalAccessFrom(enclosingClass);
DD->copyFormalAccessFrom(enclosingClass, /*sourceIsParentContext*/true);

configureImplicitSelf(*this, DD);

Expand Down
10 changes: 10 additions & 0 deletions test/Sema/struct_equatable_hashable_access.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %target-swift-frontend -print-ast %s 2>&1 | %FileCheck %s

// Check that synthesized members show up as 'fileprivate', not 'private.

// CHECK-LABEL: private struct PrivateConformer : Hashable {
private struct PrivateConformer: Hashable {
var value: Int
// CHECK-DAG: fileprivate var hashValue: Int { get }
// CHECK-DAG: @_implements(Equatable, ==(_:_:)) fileprivate static func __derived_struct_equals
}
27 changes: 27 additions & 0 deletions test/decl/protocol/special/coding/class_codable_inheritance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,30 @@ let _ = SimpleChildClass.encode(to:)
// The synthesized CodingKeys type should not be accessible from outside the
// class.
let _ = SimpleChildClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}

// Check access level issues around 'private'.
private class PrivateClass: Codable {
var x: Int = 1
}

private class PrivateClassChild: PrivateClass {}

_ = PrivateClass.init(from:)
_ = PrivateClass.encode(to:)
_ = PrivateClassChild.init(from:)
_ = PrivateClassChild.encode(to:)

_ = PrivateClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}

// Check access level issues around 'open'.
open class OpenClass: Codable {
var x: Int = 1
}
open class OpenClassChild: OpenClass {}

_ = OpenClass.init(from:)
_ = OpenClass.encode(to:)
_ = OpenClassChild.init(from:)
_ = OpenClassChild.encode(to:)

_ = OpenClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}