Skip to content

Commit 3c740e4

Browse files
committed
Don't fix access of an 'open' decl in a 'public' extension
This is reasonable to diagnose with a warning, but dropping the 'open' down to 'public' isn't the right fix, because now it's not a valid override. The declaration has to get moved to another extension instead, or the extension has to not set a default access level. This turned out to be a source compat issue because the same logic that emits the fix-it also updates the access of the member, which then resulted in "must be as accessible as the declaration it overrides" in the /same/ build. It's not immediately clear what caused this; probably something's just being validated in a different order than it was before. The change makes sense either way. Stepping back, it's weird that a warning would change how the compiler saw the code, and while we could check for 'override' explicitly, we can't know if the member might be satisfying a protocol requirement. Better to just not guess at the right answer here. rdar://problem/47557376&28493971
1 parent 1f06cd7 commit 3c740e4

File tree

6 files changed

+73
-37
lines changed

6 files changed

+73
-37
lines changed

lib/Sema/TypeCheckAttr.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,8 +1641,10 @@ void AttributeChecker::visitAccessControlAttr(AccessControlAttr *attr) {
16411641
diag::access_control_ext_member_more,
16421642
attr->getAccess(),
16431643
extAttr->getAccess());
1644-
swift::fixItAccess(diag, cast<ValueDecl>(D), defaultAccess, false,
1645-
true);
1644+
// Don't try to fix this one; it's just a warning, and fixing it can
1645+
// lead to diagnostic fights between this and "declaration must be at
1646+
// least this accessible" checking for overrides and protocol
1647+
// requirements.
16461648
return;
16471649
} else if (attr->getAccess() == defaultAccess) {
16481650
TC.diagnose(attr->getLocation(),

test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,11 @@ struct NonTrivialToCopyWrapper {
241241
struct TrivialToCopy {
242242
__unsafe_unretained id field;
243243
};
244+
245+
@interface OverrideInExtensionBase : NSObject
246+
- (void)method;
247+
- (void)accessWarning;
248+
@end
249+
250+
@interface OverrideInExtensionSub : OverrideInExtensionBase
251+
@end

test/ClangImporter/objc_override.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ class MyHashableNSObject: NSObject {
116116
}
117117
}
118118

119+
// rdar://problem/47557376
120+
// Adding an override to someone else's class in an extension like this isn't
121+
// really sound, but it's allowed in Objective-C too.
122+
extension OverrideInExtensionSub {
123+
open override func method() {}
124+
}
125+
public extension OverrideInExtensionSub {
126+
open override func accessWarning() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'public'}}
127+
}
128+
119129
// FIXME: Remove -verify-ignore-unknown.
120130
// <unknown>:0: error: unexpected note produced: overridden declaration is here
121131
// <unknown>:0: error: unexpected note produced: setter for 'boolProperty' declared here

test/Compatibility/accessibility.swift

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,17 @@ public extension PublicStruct {
8080
private func extImplPublic() {}
8181
}
8282
internal extension PublicStruct {
83-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
83+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
8484
fileprivate func extFuncInternal() {}
8585
private func extImplInternal() {}
8686
}
8787
fileprivate extension PublicStruct {
88-
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{3-10=}}
88+
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
8989
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
9090
private func extImplFilePrivate() {}
9191
}
9292
private extension PublicStruct {
93-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
93+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
9494
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
9595
private func extImplPrivate() {}
9696
}
@@ -100,17 +100,17 @@ public extension InternalStruct { // expected-error {{extension of internal stru
100100
private func extImplPublic() {}
101101
}
102102
internal extension InternalStruct {
103-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
103+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
104104
fileprivate func extFuncInternal() {}
105105
private func extImplInternal() {}
106106
}
107107
fileprivate extension InternalStruct {
108-
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{3-10=}}
108+
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
109109
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
110110
private func extImplFilePrivate() {}
111111
}
112112
private extension InternalStruct {
113-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
113+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
114114
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
115115
private func extImplPrivate() {}
116116
}
@@ -120,17 +120,17 @@ public extension FilePrivateStruct { // expected-error {{extension of fileprivat
120120
private func extImplPublic() {}
121121
}
122122
internal extension FilePrivateStruct { // expected-error {{extension of fileprivate struct cannot be declared internal}} {{1-10=}}
123-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
123+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
124124
fileprivate func extFuncInternal() {}
125125
private func extImplInternal() {}
126126
}
127127
fileprivate extension FilePrivateStruct {
128-
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{3-10=}}
128+
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
129129
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
130130
private func extImplFilePrivate() {}
131131
}
132132
private extension FilePrivateStruct {
133-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
133+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
134134
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
135135
private func extImplPrivate() {}
136136
}
@@ -140,17 +140,17 @@ public extension PrivateStruct { // expected-error {{extension of private struct
140140
private func extImplPublic() {}
141141
}
142142
internal extension PrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-10=}}
143-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
143+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
144144
fileprivate func extFuncInternal() {}
145145
private func extImplInternal() {}
146146
}
147147
fileprivate extension PrivateStruct { // expected-error {{extension of private struct cannot be declared fileprivate}} {{1-13=}}
148-
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{3-10=}}
148+
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
149149
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
150150
private func extImplFilePrivate() {}
151151
}
152152
private extension PrivateStruct {
153-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
153+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
154154
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
155155
private func extImplPrivate() {}
156156
}
@@ -175,10 +175,10 @@ public class Base {
175175

176176

177177
public extension Base {
178-
open func extMemberPublic() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'public'}}
178+
open func extMemberPublic() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'public'}} {{none}}
179179
}
180180
internal extension Base {
181-
open func extMemberInternal() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'internal'}}
181+
open func extMemberInternal() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'internal'}} {{none}}
182182
}
183183

184184
public class PublicSub: Base {

test/SILGen/accessibility_warnings.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,35 +65,35 @@ public extension PublicStruct {
6565
}
6666

6767
internal extension PublicStruct {
68-
// CHECK-DAG: sil hidden [ossa] @$s22accessibility_warnings12PublicStructV17extMemberInternalyyF
69-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
68+
// CHECK-DAG: sil [ossa] @$s22accessibility_warnings12PublicStructV17extMemberInternalyyF
69+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
7070
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings12PublicStructV15extImplInternal33_5D2F2E026754A901C0FF90C404896D02LLyyF
7171
private func extImplInternal() {}
7272
}
7373
private extension PublicStruct {
74-
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings12PublicStructV16extMemberPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
75-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
74+
// CHECK-DAG: sil [ossa] @$s22accessibility_warnings12PublicStructV16extMemberPrivateyyF
75+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
7676
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings12PublicStructV14extImplPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
7777
private func extImplPrivate() {}
7878
}
7979

8080
internal extension InternalStruct {
8181
// CHECK-DAG: sil hidden [ossa] @$s22accessibility_warnings14InternalStructV09extMemberC0yyF
82-
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{3-10=}}
82+
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
8383
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings14InternalStructV07extImplC033_5D2F2E026754A901C0FF90C404896D02LLyyF
8484
private func extImplInternal() {}
8585
}
8686
private extension InternalStruct {
87-
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings14InternalStructV16extMemberPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
88-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
87+
// CHECK-DAG: sil hidden [ossa] @$s22accessibility_warnings14InternalStructV16extMemberPrivateyyF
88+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
8989
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings14InternalStructV14extImplPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
9090
private func extImplPrivate() {}
9191
}
9292

9393

9494
private extension PrivateStruct {
9595
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings13PrivateStruct33_5D2F2E026754A901C0FF90C404896D02LLV09extMemberC0yyF
96-
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{3-10=}}
96+
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
9797
// CHECK-DAG: sil private [ossa] @$s22accessibility_warnings13PrivateStruct33_5D2F2E026754A901C0FF90C404896D02LLV07extImplC0yyF
9898
private func extImplPrivate() {}
9999
}

0 commit comments

Comments
 (0)