Skip to content

Commit ea84da3

Browse files
slavapestovslavapestov
committed
Parse: Stop synthesizing implicit getters and setters
Sema is now capable of synthesizing accessors for local properties, and SILGen can emit them, so we can remove this codepath altogether.
1 parent a1b8913 commit ea84da3

File tree

4 files changed

+53
-75
lines changed

4 files changed

+53
-75
lines changed

‎lib/Parse/ParseDecl.cpp

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4968,32 +4968,6 @@ Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
49684968
if (auto *subscript = dyn_cast<SubscriptDecl>(storage))
49694969
genericParams = subscript->getGenericParams();
49704970

4971-
// Create an implicit accessor declaration.
4972-
auto createImplicitAccessor = [&](AccessorKind kind,
4973-
AccessorDecl *funcForParams = nullptr) {
4974-
// We never use this to create addressors.
4975-
assert(kind != AccessorKind::Address &&
4976-
kind != AccessorKind::MutableAddress);
4977-
4978-
// Create the paramter list for a setter.
4979-
ParameterList *argList = nullptr;
4980-
if (kind == AccessorKind::Set) {
4981-
assert(funcForParams);
4982-
auto argLoc = funcForParams->getStartLoc();
4983-
4984-
auto argument = createSetterAccessorArgument(
4985-
argLoc, Identifier(), AccessorKind::Set, P, elementTy);
4986-
argList = ParameterList::create(P.Context, argument);
4987-
}
4988-
4989-
auto accessor = createAccessorFunc(SourceLoc(), argList,
4990-
genericParams, indices, elementTy,
4991-
staticLoc, flags, kind,
4992-
storage, &P, SourceLoc());
4993-
accessor->setImplicit();
4994-
add(accessor);
4995-
};
4996-
49974971
// If there was a problem parsing accessors, mark all parsed accessors
49984972
// as invalid to avoid tripping up later invariants.
49994973
// We also want to avoid diagnose missing accessors if something
@@ -5020,12 +4994,6 @@ Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
50204994

50214995
// Otherwise, we have either a stored or inherited observing property.
50224996
} else {
5023-
// Observing properties will have getters and setters synthesized
5024-
// by Sema. Create their prototypes now.
5025-
auto argFunc = (WillSet ? WillSet : DidSet);
5026-
createImplicitAccessor(AccessorKind::Get);
5027-
createImplicitAccessor(AccessorKind::Set, argFunc);
5028-
50294997
if (attrs.hasAttribute<OverrideAttr>()) {
50304998
return StorageImplInfo(ReadImplKind::Inherited,
50314999
WriteImplKind::InheritedWithObservers,
@@ -5066,7 +5034,7 @@ Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
50665034
isa<SubscriptDecl>(storage),
50675035
getAccessorNameForDiagnostic(mutator, /*article*/ true));
50685036
}
5069-
createImplicitAccessor(AccessorKind::Get);
5037+
50705038
readImpl = ReadImplKind::Get;
50715039

50725040
// Subscripts always have to have some sort of accessor; they can't be
@@ -5076,7 +5044,6 @@ Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
50765044
P.diagnose(LBLoc, diag::subscript_without_get);
50775045
}
50785046

5079-
createImplicitAccessor(AccessorKind::Get);
50805047
readImpl = ReadImplKind::Get;
50815048

50825049
// Otherwise, it's stored.

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,10 +2026,13 @@ static bool computeIsGetterMutating(TypeChecker &TC,
20262026

20272027
switch (storage->getReadImpl()) {
20282028
case ReadImplKind::Stored:
2029+
case ReadImplKind::Inherited:
20292030
return false;
20302031

20312032
case ReadImplKind::Get:
2032-
case ReadImplKind::Inherited:
2033+
if (!storage->getGetter())
2034+
return false;
2035+
20332036
return validateAccessorIsMutating(TC, storage->getGetter());
20342037

20352038
case ReadImplKind::Address:
@@ -2059,6 +2062,11 @@ static bool computeIsSetterMutating(TypeChecker &TC,
20592062
}
20602063
}
20612064

2065+
// By default, the setter is mutating if we have an instance member of a
2066+
// value type, but this can be overridden below.
2067+
bool result = (storage->isInstanceMember() &&
2068+
doesContextHaveValueSemantics(storage->getDeclContext()));
2069+
20622070
auto impl = storage->getImplInfo();
20632071
switch (impl.getWriteImpl()) {
20642072
case WriteImplKind::Immutable:
@@ -2067,13 +2075,15 @@ static bool computeIsSetterMutating(TypeChecker &TC,
20672075
// top-level setters are not.
20682076
// It's important that we use this logic for "immutable" storage
20692077
// in order to handle initialization of let-properties.
2070-
return storage->isInstanceMember() &&
2071-
doesContextHaveValueSemantics(storage->getDeclContext());
2078+
return result;
20722079

20732080
case WriteImplKind::StoredWithObservers:
20742081
case WriteImplKind::InheritedWithObservers:
20752082
case WriteImplKind::Set: {
2076-
auto result = validateAccessorIsMutating(TC, storage->getSetter());
2083+
auto *setter = storage->getSetter();
2084+
2085+
if (setter)
2086+
result = validateAccessorIsMutating(TC, setter);
20772087

20782088
// As a special extra check, if the user also gave us a modify
20792089
// coroutine, check that it has the same mutatingness as the setter.
@@ -2089,7 +2099,8 @@ static bool computeIsSetterMutating(TypeChecker &TC,
20892099
: SelfAccessKind::NonMutating,
20902100
modifyResult ? SelfAccessKind::NonMutating
20912101
: SelfAccessKind::Mutating);
2092-
TC.diagnose(storage->getSetter(), diag::previous_accessor, "setter", 0);
2102+
if (setter)
2103+
TC.diagnose(setter, diag::previous_accessor, "setter", 0);
20932104
modifyAccessor->setInvalid();
20942105
}
20952106
}

‎test/ParseableInterface/inlinable-function.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public struct Foo: Hashable {
3131
// CHECK: public var hasDidSet: Swift.Int {
3232
public var hasDidSet: Int {
3333
// CHECK-NEXT: @_transparent get{{$}}
34-
// CHECK-NEXT: set[[NEWVALUE]]{{$}}
34+
// CHECK-NEXT: set{{(\(value\))?}}{{$}}
3535
// CHECK-NOT: didSet
3636
didSet {
3737
print("b set to \(hasDidSet)")

‎test/SILGen/observers.swift

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,40 @@ public struct DidSetWillSetTests {
2626
}
2727

2828
public var a: Int {
29+
// This is the synthesized getter and setter for the willset/didset variable.
30+
31+
// CHECK-LABEL: sil [transparent] [serialized] [ossa] @$s9observers010DidSetWillC5TestsV1aSivg
32+
// CHECK: bb0(%0 : $DidSetWillSetTests):
33+
// CHECK-NEXT: debug_value %0
34+
// CHECK-NEXT: %2 = struct_extract %0 : $DidSetWillSetTests, #DidSetWillSetTests.a
35+
// CHECK-NEXT: return %2 : $Int{{.*}} // id: %3
36+
37+
38+
// CHECK-LABEL: sil [ossa] @$s9observers010DidSetWillC5TestsV1aSivs
39+
// CHECK: bb0(%0 : $Int, %1 : $*DidSetWillSetTests):
40+
// CHECK-NEXT: debug_value %0
41+
// CHECK-NEXT: debug_value_addr %1
42+
43+
// CHECK-NEXT: [[READ:%.*]] = begin_access [read] [unknown] %1
44+
// CHECK-NEXT: [[AADDR:%.*]] = struct_element_addr [[READ]] : $*DidSetWillSetTests, #DidSetWillSetTests.a
45+
// CHECK-NEXT: [[OLDVAL:%.*]] = load [trivial] [[AADDR]] : $*Int
46+
// CHECK-NEXT: end_access [[READ]]
47+
// CHECK-NEXT: debug_value [[OLDVAL]] : $Int, let, name "tmp"
48+
49+
// CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
50+
// CHECK-NEXT: // function_ref {{.*}}.DidSetWillSetTests.a.willset : Swift.Int
51+
// CHECK-NEXT: [[WILLSETFN:%.*]] = function_ref @$s9observers010DidSetWillC5TestsV1a{{[_0-9a-zA-Z]*}}vw
52+
// CHECK-NEXT: apply [[WILLSETFN]](%0, [[WRITE]]) : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
53+
// CHECK-NEXT: end_access [[WRITE]]
54+
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
55+
// CHECK-NEXT: [[AADDR:%.*]] = struct_element_addr [[WRITE]] : $*DidSetWillSetTests, #DidSetWillSetTests.a
56+
// CHECK-NEXT: assign %0 to [[AADDR]] : $*Int
57+
// CHECK-NEXT: end_access [[WRITE]]
58+
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
59+
// CHECK-NEXT: // function_ref {{.*}}.DidSetWillSetTests.a.didset : Swift.Int
60+
// CHECK-NEXT: [[DIDSETFN:%.*]] = function_ref @$s9observers010DidSetWillC5TestsV1a{{[_0-9a-zA-Z]*}}vW : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
61+
// CHECK-NEXT: apply [[DIDSETFN]]([[OLDVAL]], [[WRITE]]) : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
62+
2963
// CHECK-LABEL: sil private [ossa] @$s9observers010DidSetWillC5TestsV1a{{[_0-9a-zA-Z]*}}vw
3064
willSet(newA) {
3165
// CHECK: bb0(%0 : $Int, %1 : $*DidSetWillSetTests):
@@ -92,40 +126,6 @@ public struct DidSetWillSetTests {
92126
}
93127
}
94128

95-
// This is the synthesized getter and setter for the willset/didset variable.
96-
97-
// CHECK-LABEL: sil [transparent] [serialized] [ossa] @$s9observers010DidSetWillC5TestsV1aSivg
98-
// CHECK: bb0(%0 : $DidSetWillSetTests):
99-
// CHECK-NEXT: debug_value %0
100-
// CHECK-NEXT: %2 = struct_extract %0 : $DidSetWillSetTests, #DidSetWillSetTests.a
101-
// CHECK-NEXT: return %2 : $Int{{.*}} // id: %3
102-
103-
104-
// CHECK-LABEL: sil [ossa] @$s9observers010DidSetWillC5TestsV1aSivs
105-
// CHECK: bb0(%0 : $Int, %1 : $*DidSetWillSetTests):
106-
// CHECK-NEXT: debug_value %0
107-
// CHECK-NEXT: debug_value_addr %1
108-
109-
// CHECK-NEXT: [[READ:%.*]] = begin_access [read] [unknown] %1
110-
// CHECK-NEXT: [[AADDR:%.*]] = struct_element_addr [[READ]] : $*DidSetWillSetTests, #DidSetWillSetTests.a
111-
// CHECK-NEXT: [[OLDVAL:%.*]] = load [trivial] [[AADDR]] : $*Int
112-
// CHECK-NEXT: end_access [[READ]]
113-
// CHECK-NEXT: debug_value [[OLDVAL]] : $Int, let, name "tmp"
114-
115-
// CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
116-
// CHECK-NEXT: // function_ref {{.*}}.DidSetWillSetTests.a.willset : Swift.Int
117-
// CHECK-NEXT: [[WILLSETFN:%.*]] = function_ref @$s9observers010DidSetWillC5TestsV1a{{[_0-9a-zA-Z]*}}vw
118-
// CHECK-NEXT: apply [[WILLSETFN]](%0, [[WRITE]]) : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
119-
// CHECK-NEXT: end_access [[WRITE]]
120-
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
121-
// CHECK-NEXT: [[AADDR:%.*]] = struct_element_addr [[WRITE]] : $*DidSetWillSetTests, #DidSetWillSetTests.a
122-
// CHECK-NEXT: assign %0 to [[AADDR]] : $*Int
123-
// CHECK-NEXT: end_access [[WRITE]]
124-
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] %1
125-
// CHECK-NEXT: // function_ref {{.*}}.DidSetWillSetTests.a.didset : Swift.Int
126-
// CHECK-NEXT: [[DIDSETFN:%.*]] = function_ref @$s9observers010DidSetWillC5TestsV1a{{[_0-9a-zA-Z]*}}vW : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
127-
// CHECK-NEXT: apply [[DIDSETFN]]([[OLDVAL]], [[WRITE]]) : $@convention(method) (Int, @inout DidSetWillSetTests) -> ()
128-
129129
// CHECK-LABEL: sil hidden [ossa] @$s9observers010DidSetWillC5TestsV8testReadSiyF
130130
// CHECK: [[SELF:%.*]] = begin_access [read] [unknown] %0 : $*DidSetWillSetTests
131131
// CHECK-NEXT: [[PROP:%.*]] = struct_element_addr [[SELF]] : $*DidSetWillSetTests
@@ -313,7 +313,7 @@ func propertyWithDidSetTakingOldValue() {
313313

314314
// CHECK-LABEL: sil private [ossa] @$s9observers32propertyWithDidSetTakingOldValueyyF1pL_Sivs
315315
// CHECK: bb0([[ARG1:%.*]] : $Int, [[ARG2:%.*]] : @guaranteed ${ var Int }):
316-
// CHECK-NEXT: debug_value [[ARG1]] : $Int, let, name "newValue", argno 1
316+
// CHECK-NEXT: debug_value [[ARG1]] : $Int, let, name "value", argno 1
317317
// CHECK-NEXT: [[ARG2_PB:%.*]] = project_box [[ARG2]]
318318
// CHECK-NEXT: debug_value_addr [[ARG2_PB]] : $*Int, var, name "p", argno 2
319319
// CHECK-NEXT: [[READ:%.*]] = begin_access [read] [unknown] [[ARG2_PB]]

0 commit comments

Comments
 (0)