Skip to content

Commit 1c3b994

Browse files
authored
Merge pull request #73664 from DougGregor/instance-let-not-inout-in-initializer-6.0
[6.0] Consistently treat let properties as immutable within the type checker
2 parents 19da726 + cc4ab6a commit 1c3b994

18 files changed

+364
-142
lines changed

include/swift/AST/Decl.h

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5571,6 +5571,21 @@ class BuiltinTupleDecl final : public NominalTypeDecl {
55715571
}
55725572
};
55735573

5574+
/// Describes whether a particular storage declaration is mutable.
5575+
enum class StorageMutability {
5576+
/// The storage is immutable, meaning that it can neither be assigned
5577+
/// to nor passed inout.
5578+
Immutable,
5579+
5580+
/// The storage is mutable, meaning that it can be assigned and pased
5581+
/// inout.
5582+
Mutable,
5583+
5584+
/// The storage is immutable, but can be asigned for the purposes of
5585+
/// initialization.
5586+
Initializable
5587+
};
5588+
55745589
/// AbstractStorageDecl - This is the common superclass for VarDecl and
55755590
/// SubscriptDecl, representing potentially settable memory locations.
55765591
class AbstractStorageDecl : public ValueDecl {
@@ -5742,8 +5757,31 @@ class AbstractStorageDecl : public ValueDecl {
57425757
/// Determine whether references to this storage declaration may appear
57435758
/// on the left-hand side of an assignment, as the operand of a
57445759
/// `&` or 'inout' operator, or as a component in a writable key path.
5745-
bool isSettable(const DeclContext *UseDC,
5746-
const DeclRefExpr *base = nullptr) const;
5760+
bool isSettable(const DeclContext *useDC,
5761+
const DeclRefExpr *base = nullptr) const {
5762+
switch (mutability(useDC, base)) {
5763+
case StorageMutability::Immutable:
5764+
return false;
5765+
case StorageMutability::Mutable:
5766+
case StorageMutability::Initializable:
5767+
return true;
5768+
}
5769+
}
5770+
5771+
/// Determine the mutability of this storage declaration when
5772+
/// accessed from a given declaration context.
5773+
StorageMutability mutability(const DeclContext *useDC,
5774+
const DeclRefExpr *base = nullptr) const;
5775+
5776+
/// Determine the mutability of this storage declaration when
5777+
/// accessed from a given declaration context in Swift.
5778+
///
5779+
/// This method differs only from 'mutability()' in its handling of
5780+
/// 'optional' storage requirements, which lack support for direct
5781+
/// writes in Swift.
5782+
StorageMutability mutabilityInSwift(
5783+
const DeclContext *useDC,
5784+
const DeclRefExpr *base = nullptr) const;
57475785

57485786
/// Determine whether references to this storage declaration in Swift may
57495787
/// appear on the left-hand side of an assignment, as the operand of a
@@ -5752,8 +5790,16 @@ class AbstractStorageDecl : public ValueDecl {
57525790
/// This method is equivalent to \c isSettable with the exception of
57535791
/// 'optional' storage requirements, which lack support for direct writes
57545792
/// in Swift.
5755-
bool isSettableInSwift(const DeclContext *UseDC,
5756-
const DeclRefExpr *base = nullptr) const;
5793+
bool isSettableInSwift(const DeclContext *useDC,
5794+
const DeclRefExpr *base = nullptr) const {
5795+
switch (mutabilityInSwift(useDC, base)) {
5796+
case StorageMutability::Immutable:
5797+
return false;
5798+
case StorageMutability::Mutable:
5799+
case StorageMutability::Initializable:
5800+
return true;
5801+
}
5802+
}
57575803

57585804
/// Does this storage declaration have explicitly-defined accessors
57595805
/// written in the source?
@@ -6070,13 +6116,10 @@ class VarDecl : public AbstractStorageDecl {
60706116
/// precisely point to the variable type because of type aliases.
60716117
SourceRange getTypeSourceRangeForDiagnostics() const;
60726118

6073-
/// Returns whether the var is settable in the specified context: this
6074-
/// is either because it is a stored var, because it has a custom setter, or
6075-
/// is a let member in an initializer.
6076-
///
6077-
/// Pass a null context and null base to check if it's always settable.
6078-
bool isSettable(const DeclContext *UseDC,
6079-
const DeclRefExpr *base = nullptr) const;
6119+
/// Determine the mutability of this variable declaration when
6120+
/// accessed from a given declaration context.
6121+
StorageMutability mutability(const DeclContext *useDC,
6122+
const DeclRefExpr *base = nullptr) const;
60806123

60816124
/// Return the parent pattern binding that may provide an initializer for this
60826125
/// VarDecl. This returns null if there is none associated with the VarDecl.
@@ -9315,26 +9358,6 @@ findGenericParameterReferences(const ValueDecl *value, CanGenericSignature sig,
93159358
bool treatNonResultCovarianceAsInvariant,
93169359
std::optional<unsigned> skipParamIndex);
93179360

9318-
inline bool AbstractStorageDecl::isSettable(const DeclContext *UseDC,
9319-
const DeclRefExpr *base) const {
9320-
if (auto vd = dyn_cast<VarDecl>(this))
9321-
return vd->isSettable(UseDC, base);
9322-
9323-
auto sd = cast<SubscriptDecl>(this);
9324-
return sd->supportsMutation();
9325-
}
9326-
9327-
inline bool
9328-
AbstractStorageDecl::isSettableInSwift(const DeclContext *UseDC,
9329-
const DeclRefExpr *base) const {
9330-
// TODO: Writing to an optional storage requirement is not supported in Swift.
9331-
if (getAttrs().hasAttribute<OptionalAttr>()) {
9332-
return false;
9333-
}
9334-
9335-
return isSettable(UseDC, base);
9336-
}
9337-
93389361
inline void
93399362
AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) {
93409363
Accessors.setInt(accessLevel);

include/swift/Sema/ConstraintSystem.h

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2092,6 +2092,14 @@ struct ClosureIsolatedByPreconcurrency {
20922092
bool operator()(const ClosureExpr *expr) const;
20932093
};
20942094

2095+
/// Determine whether the given expression is part of the left-hand side
2096+
/// of an assignment expression.
2097+
struct IsInLeftHandSideOfAssignment {
2098+
ConstraintSystem &cs;
2099+
2100+
bool operator()(Expr *expr) const;
2101+
};
2102+
20952103
/// Describes the type produced when referencing a declaration.
20962104
struct DeclReferenceType {
20972105
/// The "opened" type, which is the type of the declaration where any
@@ -4404,7 +4412,7 @@ class ConstraintSystem {
44044412
/// \param wantInterfaceType Whether we want the interface type, if available.
44054413
Type getUnopenedTypeOfReference(VarDecl *value, Type baseType,
44064414
DeclContext *UseDC,
4407-
ConstraintLocator *memberLocator = nullptr,
4415+
ConstraintLocator *locator,
44084416
bool wantInterfaceType = false,
44094417
bool adjustForPreconcurrency = true);
44104418

@@ -4426,7 +4434,7 @@ class ConstraintSystem {
44264434
getUnopenedTypeOfReference(
44274435
VarDecl *value, Type baseType, DeclContext *UseDC,
44284436
llvm::function_ref<Type(VarDecl *)> getType,
4429-
ConstraintLocator *memberLocator = nullptr,
4437+
ConstraintLocator *locator,
44304438
bool wantInterfaceType = false,
44314439
bool adjustForPreconcurrency = true,
44324440
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType =
@@ -4436,7 +4444,10 @@ class ConstraintSystem {
44364444
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency =
44374445
[](const ClosureExpr *closure) {
44384446
return closure->isIsolatedByPreconcurrency();
4439-
});
4447+
},
4448+
llvm::function_ref<bool(Expr *)> isAssignTarget = [](Expr *) {
4449+
return false;
4450+
});
44404451

44414452
/// Given the opened type and a pile of information about a member reference,
44424453
/// determine the reference type of the member reference.

lib/AST/Decl.cpp

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3091,6 +3091,34 @@ bool AbstractStorageDecl::isSetterMutating() const {
30913091
IsSetterMutatingRequest{const_cast<AbstractStorageDecl *>(this)}, {});
30923092
}
30933093

3094+
StorageMutability
3095+
AbstractStorageDecl::mutability(const DeclContext *useDC,
3096+
const DeclRefExpr *base) const {
3097+
if (auto vd = dyn_cast<VarDecl>(this))
3098+
return vd->mutability(useDC, base);
3099+
3100+
auto sd = cast<SubscriptDecl>(this);
3101+
return sd->supportsMutation() ? StorageMutability::Mutable
3102+
: StorageMutability::Immutable;
3103+
}
3104+
3105+
/// Determine the mutability of this storage declaration when
3106+
/// accessed from a given declaration context in Swift.
3107+
///
3108+
/// This method differs only from 'mutability()' in its handling of
3109+
/// 'optional' storage requirements, which lack support for direct
3110+
/// writes in Swift.
3111+
StorageMutability
3112+
AbstractStorageDecl::mutabilityInSwift(const DeclContext *useDC,
3113+
const DeclRefExpr *base) const {
3114+
// TODO: Writing to an optional storage requirement is not supported in Swift.
3115+
if (getAttrs().hasAttribute<OptionalAttr>()) {
3116+
return StorageMutability::Immutable;
3117+
}
3118+
3119+
return mutability(useDC, base);
3120+
}
3121+
30943122
OpaqueReadOwnership AbstractStorageDecl::getOpaqueReadOwnership() const {
30953123
ASTContext &ctx = getASTContext();
30963124
return evaluateOrDefault(ctx.evaluator,
@@ -7267,32 +7295,39 @@ Type VarDecl::getTypeInContext() const {
72677295
return getDeclContext()->mapTypeIntoContext(getInterfaceType());
72687296
}
72697297

7298+
/// Translate an "is mutable" bit into a StorageMutability value.
7299+
static StorageMutability storageIsMutable(bool isMutable) {
7300+
return isMutable ? StorageMutability::Mutable
7301+
: StorageMutability::Immutable;
7302+
}
7303+
72707304
/// Returns whether the var is settable in the specified context: this
72717305
/// is either because it is a stored var, because it has a custom setter, or
72727306
/// is a let member in an initializer.
7273-
bool VarDecl::isSettable(const DeclContext *UseDC,
7274-
const DeclRefExpr *base) const {
7307+
StorageMutability
7308+
VarDecl::mutability(const DeclContext *UseDC,
7309+
const DeclRefExpr *base) const {
72757310
// Parameters are settable or not depending on their ownership convention.
72767311
if (auto *PD = dyn_cast<ParamDecl>(this))
7277-
return !PD->isImmutableInFunctionBody();
7312+
return storageIsMutable(!PD->isImmutableInFunctionBody());
72787313

72797314
// If this is a 'var' decl, then we're settable if we have storage or a
72807315
// setter.
72817316
if (!isLet()) {
72827317
if (hasInitAccessor()) {
72837318
if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(UseDC)) {
72847319
if (base && ctor->getImplicitSelfDecl() != base->getDecl())
7285-
return supportsMutation();
7286-
return true;
7320+
return storageIsMutable(supportsMutation());
7321+
return StorageMutability::Initializable;
72877322
}
72887323
}
72897324

7290-
return supportsMutation();
7325+
return storageIsMutable(supportsMutation());
72917326
}
72927327

72937328
// Static 'let's are always immutable.
72947329
if (isStatic()) {
7295-
return false;
7330+
return StorageMutability::Immutable;
72967331
}
72977332

72987333
//
@@ -7302,11 +7337,11 @@ bool VarDecl::isSettable(const DeclContext *UseDC,
73027337

73037338
// Debugger expression 'let's are initialized through a side-channel.
73047339
if (isDebuggerVar())
7305-
return false;
7340+
return StorageMutability::Immutable;
73067341

73077342
// 'let's are only ever settable from a specific DeclContext.
73087343
if (UseDC == nullptr)
7309-
return false;
7344+
return StorageMutability::Immutable;
73107345

73117346
// 'let' properties in structs/classes are only ever settable in their
73127347
// designated initializer(s) or by init accessors.
@@ -7318,61 +7353,64 @@ bool VarDecl::isSettable(const DeclContext *UseDC,
73187353
// Check whether this property is part of `initializes` list,
73197354
// and allow assignment/mutation if so. DI would be responsible
73207355
// for checking for re-assignment.
7321-
return accessor->isInitAccessor() &&
7356+
if (accessor->isInitAccessor() &&
73227357
llvm::is_contained(accessor->getInitializedProperties(),
7323-
const_cast<VarDecl *>(this));
7358+
const_cast<VarDecl *>(this)))
7359+
return StorageMutability::Initializable;
7360+
7361+
return StorageMutability::Immutable;
73247362
}
73257363

73267364
auto *CD = dyn_cast<ConstructorDecl>(UseDC);
7327-
if (!CD) return false;
7328-
7365+
if (!CD) return StorageMutability::Immutable;
7366+
73297367
auto *CDC = CD->getDeclContext();
73307368

73317369
// 'let' properties are not valid inside protocols.
73327370
if (CDC->getExtendedProtocolDecl())
7333-
return false;
7371+
return StorageMutability::Immutable;
73347372

73357373
// If this init is defined inside of the same type (or in an extension
73367374
// thereof) as the let property, then it is mutable.
73377375
if (CDC->getSelfNominalTypeDecl() !=
73387376
getDeclContext()->getSelfNominalTypeDecl())
7339-
return false;
7377+
return StorageMutability::Immutable;
73407378

73417379
if (base && CD->getImplicitSelfDecl() != base->getDecl())
7342-
return false;
7380+
return StorageMutability::Immutable;
73437381

73447382
// If this is a convenience initializer (i.e. one that calls
73457383
// self.init), then let properties are never mutable in it. They are
73467384
// only mutable in designated initializers.
73477385
auto initKindAndExpr = CD->getDelegatingOrChainedInitKind();
73487386
if (initKindAndExpr.initKind == BodyInitKind::Delegating)
7349-
return false;
7387+
return StorageMutability::Immutable;
73507388

7351-
return true;
7389+
return StorageMutability::Initializable;
73527390
}
73537391

73547392
// If the 'let' has a value bound to it but has no PBD, then it is
73557393
// already initializedand not settable.
73567394
if (getParentPatternBinding() == nullptr)
7357-
return false;
7395+
return StorageMutability::Immutable;
73587396

73597397
// If the 'let' has an explicitly written initializer with a pattern binding,
73607398
// then it isn't settable.
73617399
if (isParentInitialized())
7362-
return false;
7400+
return StorageMutability::Immutable;
73637401

73647402
// Normal lets (e.g. globals) are only mutable in the context of the
73657403
// declaration. To handle top-level code properly, we look through
73667404
// the TopLevelCode decl on the use (if present) since the vardecl may be
73677405
// one level up.
73687406
if (getDeclContext() == UseDC)
7369-
return true;
7407+
return StorageMutability::Initializable;
73707408

73717409
if (isa<TopLevelCodeDecl>(UseDC) &&
73727410
getDeclContext() == UseDC->getParent())
7373-
return true;
7411+
return StorageMutability::Initializable;
73747412

7375-
return false;
7413+
return StorageMutability::Immutable;
73767414
}
73777415

73787416
bool VarDecl::isLazilyInitializedGlobal() const {

lib/SILGen/SILGenConcurrency.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,8 @@ bool SILGenFunction::unsafelyInheritsExecutor() {
647647

648648
void ExecutorBreadcrumb::emit(SILGenFunction &SGF, SILLocation loc) {
649649
if (mustReturnToExecutor) {
650-
assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor());
650+
assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor() ||
651+
SGF.isCtorWithHopsInjectedByDefiniteInit());
651652
if (auto executor = SGF.ExpectedExecutor)
652653
SGF.B.createHopToExecutor(
653654
RegularLocation::getDebugOnlyLocation(loc, SGF.getModule()), executor,

lib/SILGen/SILGenConstructor.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,19 @@ static bool ctorHopsInjectedByDefiniteInit(ConstructorDecl *ctor,
618618
}
619619
}
620620

621+
bool SILGenFunction::isCtorWithHopsInjectedByDefiniteInit() {
622+
auto declRef = F.getDeclRef();
623+
if (!declRef || !declRef.isConstructor())
624+
return false;
625+
626+
auto ctor = dyn_cast_or_null<ConstructorDecl>(declRef.getDecl());
627+
if (!ctor)
628+
return false;
629+
630+
auto isolation = getActorIsolation(ctor);
631+
return ctorHopsInjectedByDefiniteInit(ctor, isolation);
632+
}
633+
621634
void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
622635
MagicFunctionName = SILGenModule::getMagicFunctionName(ctor);
623636

lib/SILGen/SILGenFunction.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
820820
/// the fields.
821821
void emitDeallocatingMoveOnlyDestructor(DestructorDecl *dd);
822822

823+
/// Whether we are inside a constructor whose hops are injected by
824+
/// definite initialization.
825+
bool isCtorWithHopsInjectedByDefiniteInit();
826+
823827
/// Generates code for a struct constructor.
824828
/// This allocates the new 'self' value, emits the
825829
/// body code, then returns the final initialized 'self'.

0 commit comments

Comments
 (0)