Skip to content

Commit 11f949e

Browse files
committed
Sema: Fix signature of materializeForSet with mutating getter and non-mutating setter
I admit this is an odd corner case that is unlikely to ever come up in practice. Fixes <rdar://problem/32860203>.
1 parent a685272 commit 11f949e

File tree

3 files changed

+42
-13
lines changed

3 files changed

+42
-13
lines changed

lib/AST/ASTVerifier.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2143,29 +2143,30 @@ class Verifier : public ASTWalker {
21432143
if (ASD->getSetter() &&
21442144
ASD->getSetter()->getFormalAccess() != setterAccess) {
21452145
Out << "AbstractStorageDecl's setter access is out of sync"
2146-
" with the access actually on the setter";
2146+
" with the access actually on the setter\n";
21472147
abort();
21482148
}
21492149
}
21502150

21512151
if (auto getter = ASD->getGetter()) {
21522152
if (getter->isMutating() != ASD->isGetterMutating()) {
21532153
Out << "AbstractStorageDecl::isGetterMutating is out of sync"
2154-
" with whether the getter is actually mutating";
2154+
" with whether the getter is actually mutating\n";
21552155
abort();
21562156
}
21572157
}
21582158
if (auto setter = ASD->getSetter()) {
21592159
if (setter->isMutating() != ASD->isSetterMutating()) {
21602160
Out << "AbstractStorageDecl::isSetterMutating is out of sync"
2161-
" with whether the setter is actually mutating";
2161+
" with whether the setter is actually mutating\n";
21622162
abort();
21632163
}
21642164
}
21652165
if (auto materializeForSet = ASD->getMaterializeForSetFunc()) {
2166-
if (materializeForSet->isMutating() != ASD->isSetterMutating()) {
2167-
Out << "AbstractStorageDecl::isSetterMutating is out of sync"
2168-
" with whether materializeForSet is mutating";
2166+
if (materializeForSet->isMutating() !=
2167+
(ASD->isSetterMutating() || ASD->isGetterMutating())) {
2168+
Out << "AbstractStorageDecl::is{Getter,Setter}Mutating is out of sync"
2169+
" with whether materializeForSet is mutating\n";
21692170
abort();
21702171
}
21712172
}

lib/Sema/CodeSynthesis.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,9 @@ static void maybeMarkTransparent(FuncDecl *accessor,
296296

297297
static AccessorDecl *
298298
createMaterializeForSetPrototype(AbstractStorageDecl *storage,
299-
FuncDecl *setter, TypeChecker &TC) {
299+
FuncDecl *getter,
300+
FuncDecl *setter,
301+
TypeChecker &TC) {
300302
auto &ctx = storage->getASTContext();
301303
SourceLoc loc = storage->getLoc();
302304

@@ -319,7 +321,7 @@ createMaterializeForSetPrototype(AbstractStorageDecl *storage,
319321
params.push_back(buildIndexForwardingParamList(storage, bufferElements));
320322

321323
// The accessor returns (temporary: Builtin.RawPointer,
322-
// callback: Builtin.RawPointer),
324+
// callback: Optional<Builtin.RawPointer>),
323325
// where the first pointer is the materialized address and the
324326
// second is the address of an optional callback.
325327
TupleTypeElt retElts[] = {
@@ -349,10 +351,15 @@ createMaterializeForSetPrototype(AbstractStorageDecl *storage,
349351
// Open-code the setMutating() calculation since we might run before
350352
// the setter has been type checked.
351353
Type contextTy = DC->getDeclaredInterfaceType();
352-
if (contextTy && !contextTy->hasReferenceSemantics() &&
354+
if (contextTy && !contextTy->hasReferenceSemantics()) {
355+
bool hasMutatingSetter =
353356
!setter->getAttrs().hasAttribute<NonMutatingAttr>() &&
354-
storage->isSetterMutating())
355-
materializeForSet->setSelfAccessKind(SelfAccessKind::Mutating);
357+
storage->isSetterMutating();
358+
bool hasMutatingGetter =
359+
getter->getAttrs().hasAttribute<MutatingAttr>();
360+
if (hasMutatingSetter || hasMutatingGetter)
361+
materializeForSet->setSelfAccessKind(SelfAccessKind::Mutating);
362+
}
356363

357364
materializeForSet->setStatic(storage->isStatic());
358365

@@ -847,7 +854,7 @@ static FuncDecl *addMaterializeForSet(AbstractStorageDecl *storage,
847854
}
848855

849856
auto materializeForSet = createMaterializeForSetPrototype(
850-
storage, storage->getSetter(), TC);
857+
storage, storage->getGetter(), storage->getSetter(), TC);
851858
addMemberToContextIfNeeded(materializeForSet, storage->getDeclContext(),
852859
storage->getSetter());
853860
storage->setMaterializeForSetFunc(materializeForSet);
@@ -1815,7 +1822,9 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
18151822

18161823
AccessorDecl *materializeForSet = nullptr;
18171824
if (dc->getAsNominalTypeOrNominalTypeExtensionContext())
1818-
materializeForSet = createMaterializeForSetPrototype(var, setter, TC);
1825+
materializeForSet = createMaterializeForSetPrototype(var,
1826+
getter, setter,
1827+
TC);
18191828

18201829
var->makeComputed(SourceLoc(), getter, setter, materializeForSet, SourceLoc());
18211830

test/SILGen/materializeForSet.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,25 @@ func testMaterializedSetter() {
660660
f.computed = f.computed
661661
}
662662

663+
// Odd corner case -- mutating getter, non-mutating setter
664+
protocol BackwardMutationProtocol {
665+
var value: Int {
666+
mutating get
667+
nonmutating set
668+
}
669+
}
670+
671+
struct BackwardMutation : BackwardMutationProtocol {
672+
var value: Int {
673+
mutating get { return 0 }
674+
nonmutating set { }
675+
}
676+
}
677+
678+
func doBackwardMutation(m: inout BackwardMutationProtocol) {
679+
m.value += 1
680+
}
681+
663682
// CHECK-LABEL: sil_vtable DerivedForOverride {
664683
// CHECK: #BaseForOverride.valueStored!getter.1: (BaseForOverride) -> () -> Int : @$S17materializeForSet07DerivedB8OverrideC11valueStoredSivg
665684
// CHECK: #BaseForOverride.valueStored!setter.1: (BaseForOverride) -> (Int) -> () : @$S17materializeForSet07DerivedB8OverrideC11valueStoredSivs

0 commit comments

Comments
 (0)