Skip to content

Commit 6bd7e5e

Browse files
committed
Make sure protocol witness errors don't leave the conformance context
That is, if there's a problem with a witness, and the witness comes from a different extension from the conformance (or the original type, when the conformance is on an extension), put the main diagnostic on the conformance, with a note on the witness. This involves some shuffling and rephrasing of existing diagnostics too. There's a few reasons for this change: - More context. It may not be obvious why a declaration in file A.swift needs to be marked 'public' if you can't see the conformance in B.swift. - Better locations for imported declarations. If you're checking a conformance in a source file but the witness came from an imported module, it's better to put the diagnostic on the part you have control over. (This is especially true in Xcode, which can't display diagnostics on imported declarations in the source editor.) - Plays better with batch mode. Without this change, you can have diagnostics being reported in file A.swift that are tied to a conformance declared in file B.swift. Of course the contents of A.swift also affect the diagnostic, but compiling A.swift on its own wouldn't produce the diagnostic, and so putting it there is problematic. The change does in some cases make for a worse user experience, though; if you just want to apply the changes and move on, the main diagnostic isn't in the "right place". It's the note that has the info and possible fix-it. It's also a slightly more complicated implementation.
1 parent 545b12f commit 6bd7e5e

22 files changed

+404
-189
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,6 +1553,9 @@ ERROR(type_witness_objc_generic_parameter,none,
15531553
"type %0 involving Objective-C type parameter%select{| %1}2 cannot be "
15541554
"used for associated type %3 of protocol %4",
15551555
(Type, Type, bool, DeclName, DeclName))
1556+
NOTE(witness_fix_access,none,
1557+
"mark the %0 as '%select{%error|fileprivate|internal|public|%error}1' to "
1558+
"satisfy the requirement", (DescriptiveDeclKind, AccessLevel))
15561559

15571560
ERROR(protocol_refine_access,none,
15581561
"%select{protocol must be declared %select{"
@@ -3793,9 +3796,6 @@ ERROR(availability_protocol_requires_version,
37933796
NOTE(availability_protocol_requirement_here, none,
37943797
"protocol requirement here", ())
37953798

3796-
NOTE(availability_conformance_introduced_here, none,
3797-
"conformance introduced here", ())
3798-
37993799
// This doesn't display as an availability diagnostic, but it's
38003800
// implemented there and fires when these subscripts are marked
38013801
// unavailable, so it seems appropriate to put it here.

lib/AST/ASTContext.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,6 +2250,9 @@ bool swift::fixDeclarationName(InFlightDiagnostic &diag, ValueDecl *decl,
22502250
bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl,
22512251
Optional<ObjCSelector> targetNameOpt,
22522252
bool ignoreImpliedName) {
2253+
if (decl->isImplicit())
2254+
return false;
2255+
22532256
// Subscripts cannot be renamed, so handle them directly.
22542257
if (isa<SubscriptDecl>(decl)) {
22552258
diag.fixItInsert(decl->getAttributeInsertionLoc(/*forModifier=*/false),

lib/AST/Decl.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,6 +2095,9 @@ bool ValueDecl::canInferObjCFromRequirement(ValueDecl *requirement) {
20952095
}
20962096

20972097
SourceLoc ValueDecl::getAttributeInsertionLoc(bool forModifier) const {
2098+
if (isImplicit())
2099+
return SourceLoc();
2100+
20982101
if (auto var = dyn_cast<VarDecl>(this)) {
20992102
// [attrs] var ...
21002103
// The attributes are part of the VarDecl, but the 'var' is part of the PBD.

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 181 additions & 84 deletions
Large diffs are not rendered by default.

test/ClangImporter/objc_generics_conformance.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ extension GenericClass : WithAssocOther {
2424
typealias Other = [T] // expected-error{{type 'GenericClass<T>.Other' involving Objective-C type parameter 'T' cannot be used for associated type 'Other' of protocol 'WithAssocOther'}}
2525
}
2626

27+
protocol WithAssocSeparate {
28+
associatedtype Separate
29+
}
30+
31+
extension GenericClass {
32+
typealias Separate = T // expected-note {{'Separate' declared here}}
33+
}
34+
extension GenericClass : WithAssocSeparate { // expected-error {{type 'GenericClass<T>.Separate' involving Objective-C type parameter 'T' cannot be used for associated type 'Separate' of protocol 'WithAssocSeparate'}}
35+
}
36+
2737
protocol WithAssocElement {
2838
associatedtype Element
2939
}

test/Compatibility/accessibility.swift

Lines changed: 37 additions & 32 deletions
Large diffs are not rendered by default.

test/Compatibility/accessibility_private.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,21 @@ protocol VeryImportantProto {
118118
}
119119

120120
private struct VIPPrivateType : VeryImportantProto {
121-
private typealias Assoc = Int // expected-error {{type alias 'Assoc' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}}
121+
private typealias Assoc = Int // expected-error {{type alias 'Assoc' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{none}}
122+
// expected-note@-1 {{mark the type alias as 'fileprivate' to satisfy the requirement}} {{3-10=fileprivate}}
122123
var value: Int
123124
}
124125

125126
private struct VIPPrivateProp : VeryImportantProto {
126127
typealias Assoc = Int
127-
private var value: Int // expected-error {{property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
128+
private var value: Int // expected-error {{property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{none}}
129+
// expected-note@-1 {{mark the var as 'fileprivate' to satisfy the requirement}} {{3-10=fileprivate}}
128130
}
129131

130132
private struct VIPPrivateSetProp : VeryImportantProto {
131133
typealias Assoc = Int
132-
private(set) var value: Int // expected-error {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
134+
private(set) var value: Int // expected-error {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{none}}
135+
// expected-note@-1 {{mark the var as 'fileprivate' to satisfy the requirement}} {{3-10=fileprivate}}
133136
}
134137

135138
private class VIPPrivateSetBase {
@@ -140,9 +143,11 @@ private class VIPPrivateSetSub : VIPPrivateSetBase, VeryImportantProto { // expe
140143
}
141144

142145
private class VIPPrivateSetPropBase {
143-
private(set) var value: Int = 0 // expected-error {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{3-10=fileprivate}}
146+
private(set) var value: Int = 0
147+
// expected-note@-1 {{mark the var as 'fileprivate' to satisfy the requirement}} {{3-10=fileprivate}}
144148
}
145149
private class VIPPrivateSetPropSub : VIPPrivateSetPropBase, VeryImportantProto {
150+
// expected-error@-1 {{setter for property 'value' must be as accessible as its enclosing type because it matches a requirement in protocol 'VeryImportantProto'}} {{none}}
146151
typealias Assoc = Int
147152
}
148153

test/NameBinding/accessibility.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,19 +132,22 @@ func privateInOtherFile() {}
132132

133133
#if !ACCESS_DISABLED
134134
struct ConformerByTypeAlias : TypeProto {
135-
private typealias TheType = Int // expected-error {{type alias 'TheType' must be declared internal because it matches a requirement in internal protocol 'TypeProto'}} {{3-10=internal}}
135+
private typealias TheType = Int // expected-error {{type alias 'TheType' must be declared internal because it matches a requirement in internal protocol 'TypeProto'}} {{none}}
136+
// expected-note@-1 {{mark the type alias as 'internal' to satisfy the requirement}} {{3-10=internal}}
136137
}
137138

138139
struct ConformerByLocalType : TypeProto {
139-
private struct TheType {} // expected-error {{struct 'TheType' must be declared internal because it matches a requirement in internal protocol 'TypeProto'}} {{3-10=internal}}
140+
private struct TheType {} // expected-error {{struct 'TheType' must be declared internal because it matches a requirement in internal protocol 'TypeProto'}} {{none}}
141+
// expected-note@-1 {{mark the struct as 'internal' to satisfy the requirement}} {{3-10=internal}}
140142
}
141143

142144
private struct PrivateConformerByLocalType : TypeProto {
143145
struct TheType {} // okay
144146
}
145147

146148
private struct PrivateConformerByLocalTypeBad : TypeProto {
147-
private struct TheType {} // expected-error {{struct 'TheType' must be as accessible as its enclosing type because it matches a requirement in protocol 'TypeProto'}} {{3-10=fileprivate}}
149+
private struct TheType {} // expected-error {{struct 'TheType' must be as accessible as its enclosing type because it matches a requirement in protocol 'TypeProto'}} {{none}}
150+
// expected-note@-1 {{mark the struct as 'fileprivate' to satisfy the requirement}} {{3-10=fileprivate}}
148151
}
149152
#endif
150153

0 commit comments

Comments
 (0)