Skip to content

Commit 933ae8d

Browse files
slavapestovslavapestov
authored
Merge pull request #26446 from slavapestov/cleanup-for-lazy-accessors
Cleanup in preparation for lazy accessors
2 parents a892f8a + 04e9a7c commit 933ae8d

19 files changed

+84
-73
lines changed

‎include/swift/AST/Decl.h

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2588,12 +2588,6 @@ class ValueDecl : public Decl {
25882588
void setInterfaceType(Type type);
25892589

25902590
bool hasValidSignature() const;
2591-
2592-
/// isSettable - Determine whether references to this decl may appear
2593-
/// on the left-hand side of an assignment or as the operand of a
2594-
/// `&` or 'inout' operator.
2595-
bool isSettable(const DeclContext *UseDC,
2596-
const DeclRefExpr *base = nullptr) const;
25972591

25982592
/// isInstanceMember - Determine whether this value is an instance member
25992593
/// of an enum or protocol.
@@ -4546,6 +4540,12 @@ class AbstractStorageDecl : public ValueDecl {
45464540
return getImplInfo().supportsMutation();
45474541
}
45484542

4543+
/// isSettable - Determine whether references to this decl may appear
4544+
/// on the left-hand side of an assignment or as the operand of a
4545+
/// `&` or 'inout' operator.
4546+
bool isSettable(const DeclContext *UseDC,
4547+
const DeclRefExpr *base = nullptr) const;
4548+
45494549
/// Are there any accessors for this declaration, including implicit ones?
45504550
bool hasAnyAccessors() const {
45514551
return !getAllAccessors().empty();
@@ -4647,6 +4647,10 @@ class AbstractStorageDecl : public ValueDecl {
46474647

46484648
AccessLevel getSetterFormalAccess() const;
46494649

4650+
AccessScope
4651+
getSetterFormalAccessScope(const DeclContext *useDC = nullptr,
4652+
bool treatUsableFromInlineAsPublic = false) const;
4653+
46504654
void setSetterAccess(AccessLevel accessLevel) {
46514655
assert(!Accessors.getInt().hasValue());
46524656
overwriteSetterAccess(accessLevel);
@@ -5485,9 +5489,6 @@ class SubscriptDecl : public GenericContext, public AbstractStorageDecl {
54855489
/// element types.
54865490
void computeType();
54875491

5488-
/// Returns whether the result of the subscript operation can be set.
5489-
bool isSettable() const;
5490-
54915492
/// Determine the kind of Objective-C subscripting this declaration
54925493
/// implies.
54935494
ObjCSubscriptKind getObjCSubscriptKind() const;
@@ -7098,14 +7099,13 @@ class MissingMemberDecl : public Decl {
70987099
}
70997100
};
71007101

7101-
inline bool ValueDecl::isSettable(const DeclContext *UseDC,
7102-
const DeclRefExpr *base) const {
7103-
if (auto vd = dyn_cast<VarDecl>(this)) {
7102+
inline bool AbstractStorageDecl::isSettable(const DeclContext *UseDC,
7103+
const DeclRefExpr *base) const {
7104+
if (auto vd = dyn_cast<VarDecl>(this))
71047105
return vd->isSettable(UseDC, base);
7105-
} else if (auto sd = dyn_cast<SubscriptDecl>(this)) {
7106-
return sd->isSettable();
7107-
} else
7108-
return false;
7106+
7107+
auto sd = cast<SubscriptDecl>(this);
7108+
return sd->supportsMutation();
71097109
}
71107110

71117111
inline void

‎lib/AST/Decl.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4799,6 +4799,13 @@ AbstractStorageDecl::getSetterFormalAccess() const {
47994799
AccessLevel::Private);
48004800
}
48014801

4802+
AccessScope
4803+
AbstractStorageDecl::getSetterFormalAccessScope(const DeclContext *useDC,
4804+
bool treatUsableFromInlineAsPublic) const {
4805+
return getAccessScopeForFormalAccess(this, getSetterFormalAccess(), useDC,
4806+
treatUsableFromInlineAsPublic);
4807+
}
4808+
48024809
void AbstractStorageDecl::setComputedSetter(AccessorDecl *setter) {
48034810
assert(getImplInfo().getReadImpl() == ReadImplKind::Get);
48044811
assert(!getImplInfo().supportsMutation());
@@ -5055,10 +5062,6 @@ bool VarDecl::isLazilyInitializedGlobal() const {
50555062
return !sourceFileContext->isScriptMode();
50565063
}
50575064

5058-
bool SubscriptDecl::isSettable() const {
5059-
return supportsMutation();
5060-
}
5061-
50625065
SourceRange VarDecl::getSourceRange() const {
50635066
if (auto Param = dyn_cast<ParamDecl>(this))
50645067
return Param->getSourceRange();

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6694,7 +6694,7 @@ SwiftDeclConverter::importSubscript(Decl *decl,
66946694
if (setter && existingSubscript && getterAndSetterInSameType) {
66956695
// Can we update the subscript by adding the setter?
66966696
if (existingSubscript->hasClangNode() &&
6697-
!existingSubscript->isSettable()) {
6697+
!existingSubscript->supportsMutation()) {
66986698
// Create the setter thunk.
66996699
auto setterThunk = buildSubscriptSetterDecl(
67006700
Impl, existingSubscript, setter, elementTy,

‎lib/IRGen/GenClass.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/AST/AttrKind.h"
2323
#include "swift/AST/Decl.h"
2424
#include "swift/AST/IRGenOptions.h"
25+
#include "swift/AST/LazyResolver.h"
2526
#include "swift/AST/Module.h"
2627
#include "swift/AST/Pattern.h"
2728
#include "swift/AST/PrettyStackTrace.h"
@@ -297,6 +298,9 @@ namespace {
297298
SILType classType,
298299
bool superclass) {
299300
for (VarDecl *var : theClass->getStoredProperties()) {
301+
if (!var->hasInterfaceType())
302+
IGM.Context.getLazyResolver()->resolveDeclSignature(var);
303+
300304
SILType type = classType.getFieldType(var, IGM.getSILModule());
301305

302306
// Lower the field type.

‎lib/IRGen/GenDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class CategoryInitializerVisitor
242242
llvm::Value *getterArgs[] = {classMetadata, sel, imp, types};
243243
Builder.CreateCall(class_replaceMethod, getterArgs);
244244

245-
if (subscript->isSettable()) {
245+
if (subscript->supportsMutation()) {
246246
emitObjCSetterDescriptorParts(IGM, subscript,
247247
name, types, imp);
248248
sel = Builder.CreateCall(IGM.getObjCSelRegisterNameFn(),

‎lib/IRGen/GenObjC.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1242,7 +1242,7 @@ SILFunction *irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM,
12421242
llvm::Constant *&selectorRef,
12431243
llvm::Constant *&atEncoding,
12441244
llvm::Constant *&impl) {
1245-
assert(subscript->isSettable() && "not a settable subscript?!");
1245+
assert(subscript->supportsMutation() && "not a settable subscript?!");
12461246

12471247
Selector setterSel(subscript, Selector::ForSetter);
12481248
selectorRef = IGM.getAddrOfObjCMethodName(setterSel.str());

‎lib/SIL/SILVerifier.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4944,8 +4944,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
49444944
if (F->hasForeignBody())
49454945
return;
49464946

4947-
assert(F->isAvailableExternally() &&
4948-
"external declaration of internal SILFunction not allowed");
4947+
require(F->isAvailableExternally(),
4948+
"external declaration of internal SILFunction not allowed");
49494949
// If F is an external declaration, there is nothing further to do,
49504950
// return.
49514951
return;

‎lib/SILGen/SILGen.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -613,14 +613,14 @@ static bool isEmittedOnDemand(SILModule &M, SILDeclRef constant) {
613613
if (isa<ClangModuleUnit>(dc))
614614
return true;
615615

616-
if (auto *sf = dyn_cast<SourceFile>(dc))
617-
if (M.isWholeModule() || M.getAssociatedContext() == dc)
618-
return false;
619-
620616
if (auto *func = dyn_cast<FuncDecl>(d))
621617
if (func->hasForcedStaticDispatch())
622618
return true;
623619

620+
if (auto *sf = dyn_cast<SourceFile>(dc))
621+
if (M.isWholeModule() || M.getAssociatedContext() == dc)
622+
return false;
623+
624624
return false;
625625
}
626626

‎lib/SILGen/SILGenProlog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
342342
CapturedValue capture,
343343
uint16_t ArgNo) {
344344

345-
auto *VD = capture.getDecl();
345+
auto *VD = cast<VarDecl>(capture.getDecl());
346346
SILLocation Loc(VD);
347347
Loc.markAsPrologue();
348348

‎lib/SILGen/SILGenType.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,7 +1050,8 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
10501050

10511051
void visitAccessors(AbstractStorageDecl *asd) {
10521052
for (auto *accessor : asd->getAllAccessors())
1053-
visitFuncDecl(accessor);
1053+
if (!accessor->hasForcedStaticDispatch())
1054+
visitFuncDecl(accessor);
10541055
}
10551056
};
10561057

@@ -1180,7 +1181,8 @@ class SILGenExtension : public TypeMemberVisitor<SILGenExtension> {
11801181

11811182
void visitAccessors(AbstractStorageDecl *asd) {
11821183
for (auto *accessor : asd->getAllAccessors())
1183-
visitFuncDecl(accessor);
1184+
if (!accessor->hasForcedStaticDispatch())
1185+
visitFuncDecl(accessor);
11841186
}
11851187
};
11861188

‎lib/Sema/CSDiag.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3662,7 +3662,7 @@ bool FailureDiagnosis::diagnoseSubscriptErrors(SubscriptExpr *SE,
36623662
// Classify how close this match is. Non-subscript decls don't match.
36633663
auto subscriptDecl = dyn_cast_or_null<SubscriptDecl>(cand.getDecl());
36643664
if (!subscriptDecl ||
3665-
(inAssignmentDestination && !subscriptDecl->isSettable()))
3665+
(inAssignmentDestination && !subscriptDecl->supportsMutation()))
36663666
return {CC_GeneralMismatch, {}};
36673667

36683668
// Check whether the self type matches.

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1452,7 +1452,7 @@ bool AssignmentFailure::diagnoseAsError() {
14521452
// If the underlying expression was a read-only subscript, diagnose that.
14531453
if (auto *SD = dyn_cast_or_null<SubscriptDecl>(choice->getDecl())) {
14541454
StringRef message;
1455-
if (!SD->isSettable())
1455+
if (!SD->supportsMutation())
14561456
message = "subscript is get-only";
14571457
else if (!SD->isSetterAccessibleFrom(DC))
14581458
message = "subscript setter is inaccessible";

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,6 +1272,9 @@ SynthesizeAccessorRequest::evaluate(Evaluator &evaluator,
12721272
AccessorKind kind) const {
12731273
auto &ctx = storage->getASTContext();
12741274

1275+
if (!storage->hasInterfaceType())
1276+
ctx.getLazyResolver()->resolveDeclSignature(storage);
1277+
12751278
switch (kind) {
12761279
case AccessorKind::Get:
12771280
return createGetterPrototype(storage, ctx);

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2878,7 +2878,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
28782878
checkDynamicSelfType(SD, SD->getValueInterfaceType());
28792879

28802880
if (SD->getValueInterfaceType()->hasDynamicSelfType() &&
2881-
SD->isSettable()) {
2881+
SD->supportsMutation()) {
28822882
SD->diagnose(diag::dynamic_self_in_mutable_subscript);
28832883
}
28842884
}

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -893,23 +893,20 @@ static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
893893
bool shouldDiagnose = !decl->isAccessibleFrom(scopeDC);
894894

895895
bool shouldDiagnoseSetter = false;
896-
if (!shouldDiagnose && baseDecl->isSettable(dc)){
897-
auto matchASD = cast<AbstractStorageDecl>(baseDecl);
898-
if (matchASD->isSetterAccessibleFrom(dc)) {
899-
// Match sure we've created the setter.
900-
if (!matchASD->getSetter())
901-
addExpectedOpaqueAccessorsToStorage(matchASD);
902-
903-
auto matchSetterAccessScope = matchASD->getSetter()
904-
->getFormalAccessScope(dc);
905-
auto requiredSetterAccessScope =
906-
matchSetterAccessScope.intersectWith(classAccessScope);
907-
auto setterScopeDC = requiredSetterAccessScope->getDeclContext();
908-
909-
const auto *ASD = cast<AbstractStorageDecl>(decl);
910-
shouldDiagnoseSetter =
911-
ASD->isSettable(setterScopeDC) &&
912-
!ASD->isSetterAccessibleFrom(setterScopeDC);
896+
if (auto matchASD = dyn_cast<AbstractStorageDecl>(baseDecl)) {
897+
if (!shouldDiagnose && matchASD->isSettable(dc)){
898+
if (matchASD->isSetterAccessibleFrom(dc)) {
899+
auto matchSetterAccessScope =
900+
matchASD->getSetterFormalAccessScope(dc);
901+
auto requiredSetterAccessScope =
902+
matchSetterAccessScope.intersectWith(classAccessScope);
903+
auto setterScopeDC = requiredSetterAccessScope->getDeclContext();
904+
905+
const auto *ASD = cast<AbstractStorageDecl>(decl);
906+
shouldDiagnoseSetter =
907+
ASD->isSettable(setterScopeDC) &&
908+
!ASD->isSetterAccessibleFrom(setterScopeDC);
909+
}
913910
}
914911
}
915912

@@ -1598,7 +1595,7 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
15981595
// Make sure we're not overriding a settable property with a non-settable
15991596
// one. The only reasonable semantics for this would be to inherit the
16001597
// setter but override the getter, and that would be surprising at best.
1601-
if (baseIsSettable && !override->isSettable(override->getDeclContext())) {
1598+
if (baseIsSettable && !overrideASD->isSettable(override->getDeclContext())) {
16021599
diags.diagnose(overrideASD, diag::override_mutable_with_readonly_property,
16031600
overrideASD->getBaseName().getIdentifier());
16041601
diags.diagnose(baseASD, diag::property_override_here);

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -398,16 +398,16 @@ swift::matchWitness(
398398
return RequirementMatch(witness, MatchKind::StaticNonStaticConflict);
399399

400400
// If the requirement is settable and the witness is not, reject it.
401-
if (req->isSettable(req->getDeclContext()) &&
402-
!witness->isSettable(witness->getDeclContext()))
401+
if (reqASD->isSettable(req->getDeclContext()) &&
402+
!witnessASD->isSettable(witness->getDeclContext()))
403403
return RequirementMatch(witness, MatchKind::SettableConflict);
404404

405405
// Validate that the 'mutating' bit lines up for getters and setters.
406406
if (!reqASD->isGetterMutating() && witnessASD->isGetterMutating())
407407
return RequirementMatch(getStandinForAccessor(witnessASD, AccessorKind::Get),
408408
MatchKind::MutatingConflict);
409409

410-
if (req->isSettable(req->getDeclContext())) {
410+
if (reqASD->isSettable(req->getDeclContext())) {
411411
if (!reqASD->isSetterMutating() && witnessASD->isSetterMutating())
412412
return RequirementMatch(getStandinForAccessor(witnessASD, AccessorKind::Set),
413413
MatchKind::MutatingConflict);
@@ -1197,15 +1197,17 @@ bool WitnessChecker::checkWitnessAccess(ValueDecl *requirement,
11971197
return true;
11981198
}
11991199

1200-
if (requirement->isSettable(DC)) {
1201-
*isSetter = true;
1200+
if (auto *requirementASD = dyn_cast<AbstractStorageDecl>(requirement)) {
1201+
if (requirementASD->isSettable(DC)) {
1202+
*isSetter = true;
12021203

1203-
auto ASD = cast<AbstractStorageDecl>(witness);
1204+
auto witnessASD = cast<AbstractStorageDecl>(witness);
12041205

1205-
// See above about the forConformance flag.
1206-
if (!ASD->isSetterAccessibleFrom(actualScopeToCheck.getDeclContext(),
1207-
/*forConformance=*/true))
1208-
return true;
1206+
// See above about the forConformance flag.
1207+
if (!witnessASD->isSetterAccessibleFrom(actualScopeToCheck.getDeclContext(),
1208+
/*forConformance=*/true))
1209+
return true;
1210+
}
12091211
}
12101212

12111213
return false;

‎lib/Serialization/Deserialization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3859,7 +3859,7 @@ class swift::DeclDeserializer {
38593859
return nullptr;
38603860
}
38613861

3862-
if (subscript->isSettable()) {
3862+
if (subscript->supportsMutation()) {
38633863
if (auto setterAccess = getActualAccessLevel(rawSetterAccessLevel)) {
38643864
subscript->setSetterAccess(*setterAccess);
38653865
} else {

‎lib/Serialization/Serialization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3649,7 +3649,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
36493649
uint8_t rawAccessLevel =
36503650
getRawStableAccessLevel(subscript->getFormalAccess());
36513651
uint8_t rawSetterAccessLevel = rawAccessLevel;
3652-
if (subscript->isSettable())
3652+
if (subscript->supportsMutation())
36533653
rawSetterAccessLevel =
36543654
getRawStableAccessLevel(subscript->getSetterFormalAccess());
36553655
uint8_t rawStaticSpelling =

‎test/SILGen/read_accessor.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,14 @@ protocol ReadableTitle {
181181
class OverridableGetter : ReadableTitle {
182182
var title: String = ""
183183
}
184-
// The concrete read accessor is generated on-demand and does a class dispatch to the getter.
185-
// CHECK-LABEL: sil shared [ossa] @$s13read_accessor17OverridableGetterC5titleSSvr
186-
// CHECK: class_method %0 : $OverridableGetter, #OverridableGetter.title!getter.1
187-
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableGetterC5titleSSvr'
188184
// The read witness thunk does a direct call to the concrete read accessor.
189185
// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s13read_accessor17OverridableGetterCAA13ReadableTitleA2aDP5titleSSvrTW
190186
// CHECK: function_ref @$s13read_accessor17OverridableGetterC5titleSSvr
191187
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableGetterCAA13ReadableTitleA2aDP5titleSSvrTW'
188+
// The concrete read accessor is generated on-demand and does a class dispatch to the getter.
189+
// CHECK-LABEL: sil shared [ossa] @$s13read_accessor17OverridableGetterC5titleSSvr
190+
// CHECK: class_method %0 : $OverridableGetter, #OverridableGetter.title!getter.1
191+
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableGetterC5titleSSvr'
192192

193193
protocol GettableTitle {
194194
var title: String { get }
@@ -197,11 +197,11 @@ class OverridableReader : GettableTitle {
197197
@_borrowed
198198
var title: String = ""
199199
}
200-
// The concrete getter is generated on-demand and does a class dispatch to the read accessor.
201-
// CHECK-LABEL: sil shared [ossa] @$s13read_accessor17OverridableReaderC5titleSSvg
202-
// CHECK: class_method %0 : $OverridableReader, #OverridableReader.title!read.1
203-
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableReaderC5titleSSvg'
204200
// The getter witness thunk does a direct call to the concrete getter.
205201
// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s13read_accessor17OverridableReaderCAA13GettableTitleA2aDP5titleSSvgTW
206202
// CHECK: function_ref @$s13read_accessor17OverridableReaderC5titleSSvg
207203
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableReaderCAA13GettableTitleA2aDP5titleSSvgTW'
204+
// The concrete getter is generated on-demand and does a class dispatch to the read accessor.
205+
// CHECK-LABEL: sil shared [ossa] @$s13read_accessor17OverridableReaderC5titleSSvg
206+
// CHECK: class_method %0 : $OverridableReader, #OverridableReader.title!read.1
207+
// CHECK-LABEL: // end sil function '$s13read_accessor17OverridableReaderC5titleSSvg'

0 commit comments

Comments
 (0)