Skip to content

[5.0] Don't fix access of an 'open' decl in a 'public' extension #22216

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
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
7 changes: 4 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1207,9 +1207,10 @@ WARNING(access_control_setter_redundant,none,
"%1",
(AccessLevel, DescriptiveDeclKind, AccessLevel))
WARNING(access_control_ext_member_more,none,
"declaring %select{%error|a fileprivate|an internal|a public|open}0 %1 in "
"%select{a private|a fileprivate|an internal|a public|%error}2 extension",
(AccessLevel, DescriptiveDeclKind, AccessLevel))
"'%select{%error|fileprivate|internal|public|open}0' modifier conflicts "
"with extension's default access of "
"'%select{private|fileprivate|internal|public|%error}1'",
(AccessLevel, AccessLevel))
WARNING(access_control_ext_member_redundant,none,
"'%select{%error|fileprivate|internal|public|%error}0' modifier is redundant "
"for %1 declared in %select{a private (equivalent to fileprivate)|a fileprivate"
Expand Down
9 changes: 4 additions & 5 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1633,19 +1633,18 @@ void AttributeChecker::visitAccessControlAttr(AccessControlAttr *attr) {
auto diag = TC.diagnose(attr->getLocation(),
diag::access_control_ext_member_more,
attr->getAccess(),
D->getDescriptiveKind(),
extAttr->getAccess());
swift::fixItAccess(diag, cast<ValueDecl>(D), defaultAccess, false,
true);
return;
// Don't try to fix this one; it's just a warning, and fixing it can
// lead to diagnostic fights between this and "declaration must be at
// least this accessible" checking for overrides and protocol
// requirements.
} else if (attr->getAccess() == defaultAccess) {
TC.diagnose(attr->getLocation(),
diag::access_control_ext_member_redundant,
attr->getAccess(),
D->getDescriptiveKind(),
extAttr->getAccess())
.fixItRemove(attr->getRange());
return;
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,11 @@ struct NonTrivialToCopyWrapper {
struct TrivialToCopy {
__unsafe_unretained id field;
};

@interface OverrideInExtensionBase : NSObject
- (void)method;
- (void)accessWarning;
@end

@interface OverrideInExtensionSub : OverrideInExtensionBase
@end
9 changes: 9 additions & 0 deletions test/ClangImporter/objc_override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ class MyHashableNSObject: NSObject {
}
}

// rdar://problem/47557376
// Adding an override to someone else's class in an extension like this isn't
// really sound, but it's allowed in Objective-C too.
extension OverrideInExtensionSub {
open override func method() {}
}
public extension OverrideInExtensionSub {
open override func accessWarning() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'public'}}
}

// FIXME: Remove -verify-ignore-unknown.
// <unknown>:0: error: unexpected note produced: overridden declaration is here
Expand Down
28 changes: 14 additions & 14 deletions test/Compatibility/accessibility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,17 @@ public extension PublicStruct {
private func extImplPublic() {}
}
internal extension PublicStruct {
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
fileprivate func extFuncInternal() {}
private func extImplInternal() {}
}
fileprivate extension PublicStruct {
public func extMemberFilePrivate() {} // expected-warning {{declaring a public instance method in a fileprivate extension}} {{3-10=}}
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
private func extImplFilePrivate() {}
}
private extension PublicStruct {
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
private func extImplPrivate() {}
}
Expand All @@ -100,17 +100,17 @@ public extension InternalStruct { // expected-error {{extension of internal stru
private func extImplPublic() {}
}
internal extension InternalStruct {
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
fileprivate func extFuncInternal() {}
private func extImplInternal() {}
}
fileprivate extension InternalStruct {
public func extMemberFilePrivate() {} // expected-warning {{declaring a public instance method in a fileprivate extension}} {{3-10=}}
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
private func extImplFilePrivate() {}
}
private extension InternalStruct {
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
private func extImplPrivate() {}
}
Expand All @@ -120,17 +120,17 @@ public extension FilePrivateStruct { // expected-error {{extension of fileprivat
private func extImplPublic() {}
}
internal extension FilePrivateStruct { // expected-error {{extension of fileprivate struct cannot be declared internal}} {{1-10=}}
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
fileprivate func extFuncInternal() {}
private func extImplInternal() {}
}
fileprivate extension FilePrivateStruct {
public func extMemberFilePrivate() {} // expected-warning {{declaring a public instance method in a fileprivate extension}} {{3-10=}}
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
private func extImplFilePrivate() {}
}
private extension FilePrivateStruct {
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
private func extImplPrivate() {}
}
Expand All @@ -140,17 +140,17 @@ public extension PrivateStruct { // expected-error {{extension of private struct
private func extImplPublic() {}
}
internal extension PrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-10=}}
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
fileprivate func extFuncInternal() {}
private func extImplInternal() {}
}
fileprivate extension PrivateStruct { // expected-error {{extension of private struct cannot be declared fileprivate}} {{1-13=}}
public func extMemberFilePrivate() {} // expected-warning {{declaring a public instance method in a fileprivate extension}} {{3-10=}}
public func extMemberFilePrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'fileprivate'}} {{none}}
fileprivate func extFuncFilePrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a fileprivate extension}} {{3-15=}}
private func extImplFilePrivate() {}
}
private extension PrivateStruct {
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
fileprivate func extFuncPrivate() {} // expected-warning {{'fileprivate' modifier is redundant for instance method declared in a private (equivalent to fileprivate) extension}} {{3-15=}}
private func extImplPrivate() {}
}
Expand All @@ -175,10 +175,10 @@ public class Base {


public extension Base {
open func extMemberPublic() {} // expected-warning {{declaring open instance method in a public extension}}
open func extMemberPublic() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'public'}} {{none}}
}
internal extension Base {
open func extMemberInternal() {} // expected-warning {{declaring open instance method in an internal extension}}
open func extMemberInternal() {} // expected-warning {{'open' modifier conflicts with extension's default access of 'internal'}} {{none}}
}

public class PublicSub: Base {
Expand Down
16 changes: 8 additions & 8 deletions test/SILGen/accessibility_warnings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,35 @@ public extension PublicStruct {
}

internal extension PublicStruct {
// CHECK-DAG: sil hidden @$s22accessibility_warnings12PublicStructV17extMemberInternalyyF
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
// CHECK-DAG: sil @$s22accessibility_warnings12PublicStructV17extMemberInternalyyF
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
// CHECK-DAG: sil private @$s22accessibility_warnings12PublicStructV15extImplInternal33_5D2F2E026754A901C0FF90C404896D02LLyyF
private func extImplInternal() {}
}
private extension PublicStruct {
// CHECK-DAG: sil private @$s22accessibility_warnings12PublicStructV16extMemberPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
// CHECK-DAG: sil @$s22accessibility_warnings12PublicStructV16extMemberPrivateyyF
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
// CHECK-DAG: sil private @$s22accessibility_warnings12PublicStructV14extImplPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
private func extImplPrivate() {}
}

internal extension InternalStruct {
// CHECK-DAG: sil hidden @$s22accessibility_warnings14InternalStructV09extMemberC0yyF
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}} {{3-10=}}
public func extMemberInternal() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'internal'}} {{none}}
// CHECK-DAG: sil private @$s22accessibility_warnings14InternalStructV07extImplC033_5D2F2E026754A901C0FF90C404896D02LLyyF
private func extImplInternal() {}
}
private extension InternalStruct {
// CHECK-DAG: sil private @$s22accessibility_warnings14InternalStructV16extMemberPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
// CHECK-DAG: sil hidden @$s22accessibility_warnings14InternalStructV16extMemberPrivateyyF
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
// CHECK-DAG: sil private @$s22accessibility_warnings14InternalStructV14extImplPrivate33_5D2F2E026754A901C0FF90C404896D02LLyyF
private func extImplPrivate() {}
}


private extension PrivateStruct {
// CHECK-DAG: sil private @$s22accessibility_warnings13PrivateStruct33_5D2F2E026754A901C0FF90C404896D02LLV09extMemberC0yyF
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}} {{3-10=}}
public func extMemberPrivate() {} // expected-warning {{'public' modifier conflicts with extension's default access of 'private'}} {{none}}
// CHECK-DAG: sil private @$s22accessibility_warnings13PrivateStruct33_5D2F2E026754A901C0FF90C404896D02LLV07extImplC0yyF
private func extImplPrivate() {}
}
Expand Down
Loading