Skip to content

Commit 008374b

Browse files
authored
Merge pull request #27639 from theblixguy/fix/sr-11298-third-times-a-charm
[Typechecker] Reapply fix for SR-11298
2 parents 194e334 + 2033201 commit 008374b

File tree

8 files changed

+240
-28
lines changed

8 files changed

+240
-28
lines changed

CHANGELOG.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,29 @@ Swift 5.2
5858
(foo as Magic)(5)
5959
```
6060

61+
* [SR-11298][]:
62+
63+
A class-constrained protocol extension, where the extended protocol does
64+
not impose a class constraint, will now infer the constraint implicitly.
65+
66+
```swift
67+
protocol Foo {}
68+
class Bar: Foo {
69+
var someProperty: Int = 0
70+
}
71+
72+
// Even though 'Foo' does not impose a class constraint, it is automatically
73+
// inferred due to the Self: Bar constraint.
74+
extension Foo where Self: Bar {
75+
var anotherProperty: Int {
76+
get { return someProperty }
77+
// As a result, the setter is now implicitly nonmutating, just like it would
78+
// be if 'Foo' had a class constraint.
79+
set { someProperty = newValue }
80+
}
81+
}
82+
```
83+
6184
* [SE-0253][]:
6285

6386
Values of types that declare `func callAsFunction` methods can be called
@@ -83,7 +106,7 @@ Swift 5.2
83106

84107
* [SR-4206][]:
85108

86-
A method override is no longer allowed to have a generic signature with
109+
A method override is no longer allowed to have a generic signature with
87110
requirements not imposed by the base method. For example:
88111

89112
```
@@ -7799,4 +7822,5 @@ Swift 1.0
77997822
[SR-8974]: <https://bugs.swift.org/browse/SR-8974>
78007823
[SR-9043]: <https://bugs.swift.org/browse/SR-9043>
78017824
[SR-9827]: <https://bugs.swift.org/browse/SR-9827>
7825+
[SR-11298]: <https://bugs.swift.org/browse/SR-11298>
78027826
[SR-11429]: <https://bugs.swift.org/browse/SR-11429>

include/swift/AST/DeclContext.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,13 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
261261

262262
/// Returns the kind of context this is.
263263
DeclContextKind getContextKind() const;
264-
264+
265+
/// Returns whether this context has value semantics.
266+
bool hasValueSemantics() const;
267+
268+
/// Returns whether this context is an extension constrained to a class type.
269+
bool isClassConstrainedProtocolExtension() const;
270+
265271
/// Determines whether this context is itself a local scope in a
266272
/// code block. A context that appears in such a scope, like a
267273
/// local type declaration, does not itself become a local context.

lib/AST/Decl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5628,7 +5628,7 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
56285628
if (FD && !FD->isMutating() && !FD->isImplicit() && FD->isInstanceMember()&&
56295629
!FD->getDeclContext()->getDeclaredInterfaceType()
56305630
->hasReferenceSemantics()) {
5631-
// Do not suggest the fix it in implicit getters
5631+
// Do not suggest the fix-it in implicit getters
56325632
if (auto AD = dyn_cast<AccessorDecl>(FD)) {
56335633
if (AD->isGetter() && !AD->getAccessorKeywordLoc().isValid())
56345634
return;

lib/AST/DeclContext.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,20 @@ DeclContextKind DeclContext::getContextKind() const {
10351035
llvm_unreachable("Unhandled DeclContext ASTHierarchy");
10361036
}
10371037

1038+
bool DeclContext::hasValueSemantics() const {
1039+
if (getExtendedProtocolDecl())
1040+
return !isClassConstrainedProtocolExtension();
1041+
return !getDeclaredInterfaceType()->hasReferenceSemantics();
1042+
}
1043+
1044+
bool DeclContext::isClassConstrainedProtocolExtension() const {
1045+
if (getExtendedProtocolDecl()) {
1046+
auto ED = cast<ExtensionDecl>(this);
1047+
return ED->getGenericSignature()->requiresClass(ED->getSelfInterfaceType());
1048+
}
1049+
return false;
1050+
}
1051+
10381052
SourceLoc swift::extractNearestSourceLoc(const DeclContext *dc) {
10391053
switch (dc->getContextKind()) {
10401054
case DeclContextKind::AbstractFunctionDecl:

lib/Sema/TypeCheckDecl.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2087,17 +2087,15 @@ OperatorPrecedenceGroupRequest::evaluate(Evaluator &evaluator,
20872087
return group;
20882088
}
20892089

2090-
bool swift::doesContextHaveValueSemantics(DeclContext *dc) {
2091-
if (Type contextTy = dc->getDeclaredInterfaceType())
2092-
return !contextTy->hasReferenceSemantics();
2093-
return false;
2094-
}
2095-
20962090
llvm::Expected<SelfAccessKind>
20972091
SelfAccessKindRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
20982092
if (FD->getAttrs().getAttribute<MutatingAttr>(true)) {
2099-
if (!FD->isInstanceMember() ||
2100-
!doesContextHaveValueSemantics(FD->getDeclContext())) {
2093+
if (!FD->isInstanceMember() || !FD->getDeclContext()->hasValueSemantics()) {
2094+
// If this decl is on a class-constrained protocol extension, then
2095+
// respect the explicit mutatingness. Otherwise, we would throw an
2096+
// error.
2097+
if (FD->getDeclContext()->isClassConstrainedProtocolExtension())
2098+
return SelfAccessKind::Mutating;
21012099
return SelfAccessKind::NonMutating;
21022100
}
21032101
return SelfAccessKind::Mutating;
@@ -2119,8 +2117,7 @@ SelfAccessKindRequest::evaluate(Evaluator &evaluator, FuncDecl *FD) const {
21192117
case AccessorKind::MutableAddress:
21202118
case AccessorKind::Set:
21212119
case AccessorKind::Modify:
2122-
if (AD->isInstanceMember() &&
2123-
doesContextHaveValueSemantics(AD->getDeclContext()))
2120+
if (AD->isInstanceMember() && AD->getDeclContext()->hasValueSemantics())
21242121
return SelfAccessKind::Mutating;
21252122
break;
21262123

lib/Sema/TypeCheckDecl.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ class DeclContext;
2525
class ValueDecl;
2626
class Pattern;
2727

28-
bool doesContextHaveValueSemantics(DeclContext *dc);
29-
3028
/// Walks up the override chain for \p CD until it finds an initializer that is
3129
/// required and non-implicit. If no such initializer exists, returns the
3230
/// declaration where \c required was introduced (i.e. closest to the root

lib/Sema/TypeCheckStorage.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,9 @@ void swift::validatePatternBindingEntries(TypeChecker &tc,
270270
llvm::Expected<bool>
271271
IsGetterMutatingRequest::evaluate(Evaluator &evaluator,
272272
AbstractStorageDecl *storage) const {
273-
bool result = (!storage->isStatic() &&
274-
doesContextHaveValueSemantics(storage->getDeclContext()));
273+
auto storageDC = storage->getDeclContext();
274+
bool result = (!storage->isStatic() && storageDC->isTypeContext() &&
275+
storageDC->hasValueSemantics());
275276

276277
// 'lazy' overrides the normal accessor-based rules and heavily
277278
// restricts what accessors can be used. The getter is considered
@@ -299,7 +300,7 @@ IsGetterMutatingRequest::evaluate(Evaluator &evaluator,
299300

300301
// Protocol requirements are always written as '{ get }' or '{ get set }';
301302
// the @_borrowed attribute determines if getReadImpl() becomes Get or Read.
302-
if (isa<ProtocolDecl>(storage->getDeclContext()))
303+
if (isa<ProtocolDecl>(storageDC))
303304
return checkMutability(AccessorKind::Get);
304305

305306
switch (storage->getReadImpl()) {
@@ -325,8 +326,9 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator,
325326
AbstractStorageDecl *storage) const {
326327
// By default, the setter is mutating if we have an instance member of a
327328
// value type, but this can be overridden below.
328-
bool result = (!storage->isStatic() &&
329-
doesContextHaveValueSemantics(storage->getDeclContext()));
329+
auto storageDC = storage->getDeclContext();
330+
bool result = (!storage->isStatic() && storageDC->isTypeContext() &&
331+
storageDC->hasValueSemantics());
330332

331333
// If we have an attached property wrapper, the setter is mutating
332334
// or not based on the composition of the wrappers.

test/decl/ext/extensions.swift

Lines changed: 179 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,31 +126,202 @@ struct WrapperContext {
126126
}
127127

128128
// Class-constrained extension where protocol does not impose class requirement
129+
// SR-11298
129130

130131
protocol DoesNotImposeClassReq_1 {}
131-
132+
132133
class JustAClass: DoesNotImposeClassReq_1 {
133134
var property: String = ""
134135
}
135136

136137
extension DoesNotImposeClassReq_1 where Self: JustAClass {
137-
var wrappingProperty: String {
138+
var wrappingProperty1: String {
139+
get { return property }
140+
set { property = newValue } // Okay
141+
}
142+
143+
var wrappingProperty2: String {
138144
get { return property }
139-
set { property = newValue }
145+
nonmutating set { property = newValue } // Okay
146+
}
147+
148+
var wrappingProperty3: String {
149+
get { return property }
150+
mutating set { property = newValue } // Okay
151+
}
152+
153+
mutating func foo() {
154+
property = "" // Okay
155+
wrappingProperty1 = "" // Okay
156+
wrappingProperty2 = "" // Okay
157+
wrappingProperty3 = "" // Okay
158+
}
159+
160+
func bar() { // expected-note {{mark method 'mutating' to make 'self' mutable}}{{3-3=mutating }}
161+
property = "" // Okay
162+
wrappingProperty1 = "" // Okay
163+
wrappingProperty2 = "" // Okay
164+
wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
165+
}
166+
167+
nonmutating func baz() { // expected-note {{mark method 'mutating' to make 'self' mutable}}{{3-14=mutating}}
168+
property = "" // Okay
169+
wrappingProperty1 = "" // Okay
170+
wrappingProperty2 = "" // Okay
171+
wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
140172
}
141173
}
142-
143-
let instanceOfJustAClass = JustAClass() // expected-note {{change 'let' to 'var' to make it mutable}}
144-
instanceOfJustAClass.wrappingProperty = "" // expected-error {{cannot assign to property: 'instanceOfJustAClass' is a 'let' constant}}
174+
175+
let instanceOfJustAClass1 = JustAClass() // expected-note 2{{change 'let' to 'var' to make it mutable}}
176+
instanceOfJustAClass1.wrappingProperty1 = "" // Okay
177+
instanceOfJustAClass1.wrappingProperty2 = "" // Okay
178+
instanceOfJustAClass1.wrappingProperty3 = "" // expected-error {{cannot assign to property: 'instanceOfJustAClass1' is a 'let' constant}}
179+
instanceOfJustAClass1.foo() // expected-error {{cannot use mutating member on immutable value: 'instanceOfJustAClass1' is a 'let' constant}}
180+
instanceOfJustAClass1.bar() // Okay
181+
instanceOfJustAClass1.baz() // Okay
182+
183+
var instanceOfJustAClass2 = JustAClass()
184+
instanceOfJustAClass2.foo() // Okay
145185

146186
protocol DoesNotImposeClassReq_2 {
147187
var property: String { get set }
148188
}
149189

150190
extension DoesNotImposeClassReq_2 where Self : AnyObject {
151-
var wrappingProperty: String {
191+
var wrappingProperty1: String {
152192
get { property }
153-
set { property = newValue } // Okay
193+
set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
194+
// expected-note@-1 {{mark accessor 'mutating' to make 'self' mutable}}{{5-5=mutating }}
195+
}
196+
197+
var wrappingProperty2: String {
198+
get { property }
199+
nonmutating set { property = newValue } // expected-error {{cannot assign to property: 'self' is immutable}}
200+
// expected-note@-1 {{mark accessor 'mutating' to make 'self' mutable}}{{5-16=mutating}}
201+
}
202+
203+
var wrappingProperty3: String {
204+
get { property }
205+
mutating set { property = newValue } // Okay
206+
}
207+
208+
mutating func foo() {
209+
property = "" // Okay
210+
wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
211+
wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
212+
wrappingProperty3 = "" // Okay
213+
}
214+
215+
func bar() { // expected-note 2{{mark method 'mutating' to make 'self' mutable}}{{3-3=mutating }}
216+
property = "" // expected-error {{cannot assign to property: 'self' is immutable}}
217+
wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
218+
wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
219+
wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
220+
}
221+
222+
nonmutating func baz() { // expected-note 2{{mark method 'mutating' to make 'self' mutable}}{{3-14=mutating}}
223+
property = "" // expected-error {{cannot assign to property: 'self' is immutable}}
224+
wrappingProperty1 = "" // Okay (the error is on the setter declaration above)
225+
wrappingProperty2 = "" // Okay (the error is on the setter declaration above)
226+
wrappingProperty3 = "" // expected-error {{cannot assign to property: 'self' is immutable}}
227+
}
228+
}
229+
230+
protocol DoesNotImposeClassReq_3 {
231+
var someProperty: Int { get set }
232+
}
233+
234+
class JustAClass1: DoesNotImposeClassReq_3 {
235+
var someProperty = 0
236+
}
237+
238+
extension DoesNotImposeClassReq_3 where Self: JustAClass1 {
239+
var anotherProperty1: Int {
240+
get { return someProperty }
241+
set { someProperty = newValue } // Okay
242+
}
243+
244+
var anotherProperty2: Int {
245+
get { return someProperty }
246+
mutating set { someProperty = newValue } // Okay
247+
}
248+
}
249+
250+
let justAClass1 = JustAClass1() // expected-note {{change 'let' to 'var' to make it mutable}}
251+
justAClass1.anotherProperty1 = 1234 // Okay
252+
justAClass1.anotherProperty2 = 4321 // expected-error {{cannot assign to property: 'justAClass1' is a 'let' constant}}
253+
254+
protocol ImposeClassReq1: AnyObject {
255+
var someProperty: Int { get set }
256+
}
257+
258+
class JustAClass2: ImposeClassReq1 {
259+
var someProperty = 0
260+
}
261+
262+
extension ImposeClassReq1 where Self: AnyObject {
263+
var wrappingProperty1: Int {
264+
get { return someProperty }
265+
set { someProperty = newValue }
266+
}
267+
268+
var wrappingProperty2: Int {
269+
get { return someProperty }
270+
mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
271+
}
272+
273+
mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
274+
someProperty = 1
275+
}
276+
277+
nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
278+
someProperty = 2
279+
}
280+
281+
func baz() { // Okay
282+
someProperty = 3
283+
}
284+
}
285+
286+
extension ImposeClassReq1 {
287+
var wrappingProperty3: Int {
288+
get { return someProperty }
289+
set { someProperty = newValue }
290+
}
291+
}
292+
293+
let justAClass2 = JustAClass2() // expected-note {{change 'let' to 'var' to make it mutable}}
294+
justAClass2.wrappingProperty1 = 9876 // Okay
295+
justAClass2.wrappingProperty3 = 0987 // Okay
296+
justAClass2.foo() // expected-error {{cannot use mutating member on immutable value: 'justAClass2' is a 'let' constant}}
297+
justAClass2.bar() // Okay as well (complains about explicit nonmutating on decl)
298+
justAClass2.baz() // Okay
299+
300+
protocol ImposeClassReq2: AnyObject {
301+
var someProperty: Int { get set }
302+
}
303+
304+
extension ImposeClassReq2 {
305+
var wrappingProperty1: Int {
306+
get { return someProperty }
307+
set { someProperty = newValue }
308+
}
309+
310+
var wrappingProperty2: Int {
311+
get { return someProperty }
312+
mutating set { someProperty = newValue } // expected-error {{'mutating' isn't valid on methods in classes or class-bound protocols}}
313+
}
314+
315+
mutating func foo() { // expected-error {{mutating' isn't valid on methods in classes or class-bound protocols}}
316+
someProperty = 1
317+
}
318+
319+
nonmutating func bar() { // expected-error {{'nonmutating' isn't valid on methods in classes or class-bound protocols}}
320+
someProperty = 2
321+
}
322+
323+
func baz() { // Okay
324+
someProperty = 3
154325
}
155326
}
156327

0 commit comments

Comments
 (0)