Skip to content

[6.0] Consistently treat let properties as immutable within the type checker #73664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 54 additions & 31 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5571,6 +5571,21 @@ class BuiltinTupleDecl final : public NominalTypeDecl {
}
};

/// Describes whether a particular storage declaration is mutable.
enum class StorageMutability {
/// The storage is immutable, meaning that it can neither be assigned
/// to nor passed inout.
Immutable,

/// The storage is mutable, meaning that it can be assigned and pased
/// inout.
Mutable,

/// The storage is immutable, but can be asigned for the purposes of
/// initialization.
Initializable
};

/// AbstractStorageDecl - This is the common superclass for VarDecl and
/// SubscriptDecl, representing potentially settable memory locations.
class AbstractStorageDecl : public ValueDecl {
Expand Down Expand Up @@ -5742,8 +5757,31 @@ class AbstractStorageDecl : public ValueDecl {
/// Determine whether references to this storage declaration may appear
/// on the left-hand side of an assignment, as the operand of a
/// `&` or 'inout' operator, or as a component in a writable key path.
bool isSettable(const DeclContext *UseDC,
const DeclRefExpr *base = nullptr) const;
bool isSettable(const DeclContext *useDC,
const DeclRefExpr *base = nullptr) const {
switch (mutability(useDC, base)) {
case StorageMutability::Immutable:
return false;
case StorageMutability::Mutable:
case StorageMutability::Initializable:
return true;
}
}

/// Determine the mutability of this storage declaration when
/// accessed from a given declaration context.
StorageMutability mutability(const DeclContext *useDC,
const DeclRefExpr *base = nullptr) const;

/// Determine the mutability of this storage declaration when
/// accessed from a given declaration context in Swift.
///
/// This method differs only from 'mutability()' in its handling of
/// 'optional' storage requirements, which lack support for direct
/// writes in Swift.
StorageMutability mutabilityInSwift(
const DeclContext *useDC,
const DeclRefExpr *base = nullptr) const;

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

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

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

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

inline bool AbstractStorageDecl::isSettable(const DeclContext *UseDC,
const DeclRefExpr *base) const {
if (auto vd = dyn_cast<VarDecl>(this))
return vd->isSettable(UseDC, base);

auto sd = cast<SubscriptDecl>(this);
return sd->supportsMutation();
}

inline bool
AbstractStorageDecl::isSettableInSwift(const DeclContext *UseDC,
const DeclRefExpr *base) const {
// TODO: Writing to an optional storage requirement is not supported in Swift.
if (getAttrs().hasAttribute<OptionalAttr>()) {
return false;
}

return isSettable(UseDC, base);
}

inline void
AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) {
Accessors.setInt(accessLevel);
Expand Down
17 changes: 14 additions & 3 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,14 @@ struct ClosureIsolatedByPreconcurrency {
bool operator()(const ClosureExpr *expr) const;
};

/// Determine whether the given expression is part of the left-hand side
/// of an assignment expression.
struct IsInLeftHandSideOfAssignment {
ConstraintSystem &cs;

bool operator()(Expr *expr) const;
};

/// Describes the type produced when referencing a declaration.
struct DeclReferenceType {
/// The "opened" type, which is the type of the declaration where any
Expand Down Expand Up @@ -4404,7 +4412,7 @@ class ConstraintSystem {
/// \param wantInterfaceType Whether we want the interface type, if available.
Type getUnopenedTypeOfReference(VarDecl *value, Type baseType,
DeclContext *UseDC,
ConstraintLocator *memberLocator = nullptr,
ConstraintLocator *locator,
bool wantInterfaceType = false,
bool adjustForPreconcurrency = true);

Expand All @@ -4426,7 +4434,7 @@ class ConstraintSystem {
getUnopenedTypeOfReference(
VarDecl *value, Type baseType, DeclContext *UseDC,
llvm::function_ref<Type(VarDecl *)> getType,
ConstraintLocator *memberLocator = nullptr,
ConstraintLocator *locator,
bool wantInterfaceType = false,
bool adjustForPreconcurrency = true,
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType =
Expand All @@ -4436,7 +4444,10 @@ class ConstraintSystem {
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency =
[](const ClosureExpr *closure) {
return closure->isIsolatedByPreconcurrency();
});
},
llvm::function_ref<bool(Expr *)> isAssignTarget = [](Expr *) {
return false;
});

/// Given the opened type and a pile of information about a member reference,
/// determine the reference type of the member reference.
Expand Down
84 changes: 61 additions & 23 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3091,6 +3091,34 @@ bool AbstractStorageDecl::isSetterMutating() const {
IsSetterMutatingRequest{const_cast<AbstractStorageDecl *>(this)}, {});
}

StorageMutability
AbstractStorageDecl::mutability(const DeclContext *useDC,
const DeclRefExpr *base) const {
if (auto vd = dyn_cast<VarDecl>(this))
return vd->mutability(useDC, base);

auto sd = cast<SubscriptDecl>(this);
return sd->supportsMutation() ? StorageMutability::Mutable
: StorageMutability::Immutable;
}

/// Determine the mutability of this storage declaration when
/// accessed from a given declaration context in Swift.
///
/// This method differs only from 'mutability()' in its handling of
/// 'optional' storage requirements, which lack support for direct
/// writes in Swift.
StorageMutability
AbstractStorageDecl::mutabilityInSwift(const DeclContext *useDC,
const DeclRefExpr *base) const {
// TODO: Writing to an optional storage requirement is not supported in Swift.
if (getAttrs().hasAttribute<OptionalAttr>()) {
return StorageMutability::Immutable;
}

return mutability(useDC, base);
}

OpaqueReadOwnership AbstractStorageDecl::getOpaqueReadOwnership() const {
ASTContext &ctx = getASTContext();
return evaluateOrDefault(ctx.evaluator,
Expand Down Expand Up @@ -7267,32 +7295,39 @@ Type VarDecl::getTypeInContext() const {
return getDeclContext()->mapTypeIntoContext(getInterfaceType());
}

/// Translate an "is mutable" bit into a StorageMutability value.
static StorageMutability storageIsMutable(bool isMutable) {
return isMutable ? StorageMutability::Mutable
: StorageMutability::Immutable;
}

/// Returns whether the var is settable in the specified context: this
/// is either because it is a stored var, because it has a custom setter, or
/// is a let member in an initializer.
bool VarDecl::isSettable(const DeclContext *UseDC,
const DeclRefExpr *base) const {
StorageMutability
VarDecl::mutability(const DeclContext *UseDC,
const DeclRefExpr *base) const {
// Parameters are settable or not depending on their ownership convention.
if (auto *PD = dyn_cast<ParamDecl>(this))
return !PD->isImmutableInFunctionBody();
return storageIsMutable(!PD->isImmutableInFunctionBody());

// If this is a 'var' decl, then we're settable if we have storage or a
// setter.
if (!isLet()) {
if (hasInitAccessor()) {
if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(UseDC)) {
if (base && ctor->getImplicitSelfDecl() != base->getDecl())
return supportsMutation();
return true;
return storageIsMutable(supportsMutation());
return StorageMutability::Initializable;
}
}

return supportsMutation();
return storageIsMutable(supportsMutation());
}

// Static 'let's are always immutable.
if (isStatic()) {
return false;
return StorageMutability::Immutable;
}

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

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

// 'let's are only ever settable from a specific DeclContext.
if (UseDC == nullptr)
return false;
return StorageMutability::Immutable;

// 'let' properties in structs/classes are only ever settable in their
// designated initializer(s) or by init accessors.
Expand All @@ -7318,61 +7353,64 @@ bool VarDecl::isSettable(const DeclContext *UseDC,
// Check whether this property is part of `initializes` list,
// and allow assignment/mutation if so. DI would be responsible
// for checking for re-assignment.
return accessor->isInitAccessor() &&
if (accessor->isInitAccessor() &&
llvm::is_contained(accessor->getInitializedProperties(),
const_cast<VarDecl *>(this));
const_cast<VarDecl *>(this)))
return StorageMutability::Initializable;

return StorageMutability::Immutable;
}

auto *CD = dyn_cast<ConstructorDecl>(UseDC);
if (!CD) return false;
if (!CD) return StorageMutability::Immutable;

auto *CDC = CD->getDeclContext();

// 'let' properties are not valid inside protocols.
if (CDC->getExtendedProtocolDecl())
return false;
return StorageMutability::Immutable;

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

if (base && CD->getImplicitSelfDecl() != base->getDecl())
return false;
return StorageMutability::Immutable;

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

return true;
return StorageMutability::Initializable;
}

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

// If the 'let' has an explicitly written initializer with a pattern binding,
// then it isn't settable.
if (isParentInitialized())
return false;
return StorageMutability::Immutable;

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

if (isa<TopLevelCodeDecl>(UseDC) &&
getDeclContext() == UseDC->getParent())
return true;
return StorageMutability::Initializable;

return false;
return StorageMutability::Immutable;
}

bool VarDecl::isLazilyInitializedGlobal() const {
Expand Down
3 changes: 2 additions & 1 deletion lib/SILGen/SILGenConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,8 @@ bool SILGenFunction::unsafelyInheritsExecutor() {

void ExecutorBreadcrumb::emit(SILGenFunction &SGF, SILLocation loc) {
if (mustReturnToExecutor) {
assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor());
assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor() ||
SGF.isCtorWithHopsInjectedByDefiniteInit());
if (auto executor = SGF.ExpectedExecutor)
SGF.B.createHopToExecutor(
RegularLocation::getDebugOnlyLocation(loc, SGF.getModule()), executor,
Expand Down
13 changes: 13 additions & 0 deletions lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,19 @@ static bool ctorHopsInjectedByDefiniteInit(ConstructorDecl *ctor,
}
}

bool SILGenFunction::isCtorWithHopsInjectedByDefiniteInit() {
auto declRef = F.getDeclRef();
if (!declRef || !declRef.isConstructor())
return false;

auto ctor = dyn_cast_or_null<ConstructorDecl>(declRef.getDecl());
if (!ctor)
return false;

auto isolation = getActorIsolation(ctor);
return ctorHopsInjectedByDefiniteInit(ctor, isolation);
}

void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
MagicFunctionName = SILGenModule::getMagicFunctionName(ctor);

Expand Down
4 changes: 4 additions & 0 deletions lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
/// the fields.
void emitDeallocatingMoveOnlyDestructor(DestructorDecl *dd);

/// Whether we are inside a constructor whose hops are injected by
/// definite initialization.
bool isCtorWithHopsInjectedByDefiniteInit();

/// Generates code for a struct constructor.
/// This allocates the new 'self' value, emits the
/// body code, then returns the final initialized 'self'.
Expand Down
Loading