Skip to content

Commit 3cf0063

Browse files
authored
AST-verify that 'open' is only used on classes and overridable members (swiftlang#15996)
...and fix places where it was being used inappropriately. - Don't use 'open' on non-class members in the importer. - Use the existing 'copyFormalAccessFrom' instead of an ad hoc version for synthesized typealiases for protocol conformances. (This can change 'internal' down to 'fileprivate', but only where the enclosing type was already 'private' or 'fileprivate'.) - Fix 'copyFormalAccessFrom' to not copy '@usableFromInline' onto declarations that don't support it (namely, the above typealiases). This should have no visible effect in practice.
1 parent 989a299 commit 3cf0063

File tree

6 files changed

+24
-15
lines changed

6 files changed

+24
-15
lines changed

include/swift/AST/Decl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6703,7 +6703,8 @@ inline bool ValueDecl::isImportAsMember() const {
67036703
inline bool Decl::isPotentiallyOverridable() const {
67046704
if (isa<VarDecl>(this) ||
67056705
isa<SubscriptDecl>(this) ||
6706-
isa<FuncDecl>(this)) {
6706+
isa<FuncDecl>(this) ||
6707+
isa<DestructorDecl>(this)) {
67076708
return getDeclContext()->getAsClassOrClassExtensionContext();
67086709
} else {
67096710
return false;

lib/AST/ASTVerifier.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,11 +2114,20 @@ class Verifier : public ASTWalker {
21142114
}
21152115

21162116
void verifyChecked(ValueDecl *VD) {
2117-
if (!VD->hasAccess() && !VD->getDeclContext()->isLocalContext() &&
2118-
!isa<GenericTypeParamDecl>(VD) && !isa<ParamDecl>(VD)) {
2119-
dumpRef(VD);
2120-
Out << " does not have access";
2121-
abort();
2117+
if (VD->hasAccess()) {
2118+
if (VD->getFormalAccess() == AccessLevel::Open &&
2119+
!isa<ClassDecl>(VD) && !VD->isPotentiallyOverridable()) {
2120+
Out << "decl cannot be 'open'\n";
2121+
VD->dump(Out);
2122+
abort();
2123+
}
2124+
} else {
2125+
if (!VD->getDeclContext()->isLocalContext() &&
2126+
!isa<GenericTypeParamDecl>(VD) && !isa<ParamDecl>(VD)) {
2127+
dumpRef(VD);
2128+
Out << " does not have access";
2129+
abort();
2130+
}
21222131
}
21232132

21242133
// Make sure that there are no archetypes in the interface type.

lib/AST/Decl.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2282,7 +2282,8 @@ void ValueDecl::copyFormalAccessFrom(const ValueDecl *source,
22822282
// Inherit the @usableFromInline attribute.
22832283
if (source->getAttrs().hasAttribute<UsableFromInlineAttr>() &&
22842284
!getAttrs().hasAttribute<UsableFromInlineAttr>() &&
2285-
!getAttrs().hasAttribute<InlinableAttr>()) {
2285+
!getAttrs().hasAttribute<InlinableAttr>() &&
2286+
DeclAttribute::canAttributeAppearOnDecl(DAK_UsableFromInline, this)) {
22862287
auto &ctx = getASTContext();
22872288
auto *clonedAttr = new (ctx) UsableFromInlineAttr(/*implicit=*/true);
22882289
getAttrs().add(clonedAttr);

lib/ClangImporter/ImportDecl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ static bool isInSystemModule(DeclContext *D) {
105105
return cast<ClangModuleUnit>(D->getModuleScopeContext())->isSystemModule();
106106
}
107107

108-
static AccessLevel getOverridableAccessLevel(DeclContext *dc) {
109-
return (dc->getAsProtocolOrProtocolExtensionContext()
110-
? AccessLevel::Public : AccessLevel::Open);
108+
static AccessLevel getOverridableAccessLevel(const DeclContext *dc) {
109+
return (dc->getAsClassOrClassExtensionContext()
110+
? AccessLevel::Open : AccessLevel::Public);
111111
}
112112

113113
/// Create a typedpattern(namedpattern(decl))

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,9 +2178,7 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
21782178
// a typealias added for an internal protocol shouldn't need to be
21792179
// public---but that can be problematic if the same type conforms to two
21802180
// protocols with different access levels.
2181-
AccessLevel aliasAccess = nominal->getFormalAccess();
2182-
aliasAccess = std::max(aliasAccess, AccessLevel::Internal);
2183-
aliasDecl->setAccess(aliasAccess);
2181+
aliasDecl->copyFormalAccessFrom(nominal, /*sourceIsParentContext*/true);
21842182

21852183
if (nominal == DC) {
21862184
nominal->addMember(aliasDecl);

test/attr/accessibility_print.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,8 @@ public class IC_PublicAssocTypeImpl: IA_PublicAssocTypeProto, IB_FilePrivateAsso
345345
private class ID_PrivateAssocTypeImpl: IA_PublicAssocTypeProto, IB_FilePrivateAssocTypeProto {
346346
public var publicValue: Int = 0
347347
public var filePrivateValue: Int = 0
348-
// CHECK-DAG: {{^}} internal typealias PublicValue
349-
// CHECK-DAG: {{^}} internal typealias FilePrivateValue
348+
// CHECK-DAG: {{^}} fileprivate typealias PublicValue
349+
// CHECK-DAG: {{^}} fileprivate typealias FilePrivateValue
350350
} // CHECK: {{^[}]}}
351351

352352
// CHECK-LABEL: class MultipleAttributes {

0 commit comments

Comments
 (0)