Merge pull request #73909 from DougGregor/let-instance-property-mutability-fix

Ensure that we do not turn rvalues into lvalues

Author: DougGregor

include/swift/AST/Decl.h

@@ -5773,9 +5773,8 @@ 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 {
-    switch (mutability(useDC, base)) {
+  bool isSettable(const DeclContext *useDC) const {
+    switch (mutability(useDC)) {
       case StorageMutability::Immutable:
         return false;
       case StorageMutability::Mutable:
@@ -5786,8 +5785,9 @@ class AbstractStorageDecl : public ValueDecl {
 
   /// Determine the mutability of this storage declaration when
   /// accessed from a given declaration context.
-  StorageMutability mutability(const DeclContext *useDC,
-                               const DeclRefExpr *base = nullptr) const;
+  StorageMutability mutability(
+      const DeclContext *useDC,
+      std::optional<const DeclRefExpr *> base = std::nullopt) const;
 
   /// Determine the mutability of this storage declaration when
   /// accessed from a given declaration context in Swift.
@@ -5797,7 +5797,7 @@ class AbstractStorageDecl : public ValueDecl {
   /// writes in Swift.
   StorageMutability mutabilityInSwift(
       const DeclContext *useDC,
-      const DeclRefExpr *base = nullptr) const;
+      std::optional<const DeclRefExpr *> base = std::nullopt) 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
@@ -5806,9 +5806,8 @@ 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 {
-    switch (mutabilityInSwift(useDC, base)) {
+  bool isSettableInSwift(const DeclContext *useDC) const {
+    switch (mutabilityInSwift(useDC)) {
       case StorageMutability::Immutable:
         return false;
       case StorageMutability::Mutable:
@@ -6137,8 +6136,9 @@ class VarDecl : public AbstractStorageDecl {
 
   /// Determine the mutability of this variable declaration when
   /// accessed from a given declaration context.
-  StorageMutability mutability(const DeclContext *useDC,
-                               const DeclRefExpr *base = nullptr) const;
+  StorageMutability mutability(
+      const DeclContext *useDC,
+      std::optional<const DeclRefExpr *> base = std::nullopt) 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.

lib/AST/Decl.cpp

@@ -3087,7 +3087,7 @@ bool AbstractStorageDecl::isSetterMutating() const {
 
 StorageMutability 
 AbstractStorageDecl::mutability(const DeclContext *useDC,
-                                const DeclRefExpr *base) const {
+                                std::optional<const DeclRefExpr *> base ) const {
   if (auto vd = dyn_cast<VarDecl>(this))
     return vd->mutability(useDC, base);
 
@@ -3103,8 +3103,10 @@ AbstractStorageDecl::mutability(const DeclContext *useDC,
 /// 'optional' storage requirements, which lack support for direct
 /// writes in Swift.
 StorageMutability
-AbstractStorageDecl::mutabilityInSwift(const DeclContext *useDC,
-                                       const DeclRefExpr *base) const {
+AbstractStorageDecl::mutabilityInSwift(
+    const DeclContext *useDC,
+    std::optional<const DeclRefExpr *> base
+) const {
   // TODO: Writing to an optional storage requirement is not supported in Swift.
   if (getAttrs().hasAttribute<OptionalAttr>()) {
     return StorageMutability::Immutable;
@@ -7392,7 +7394,7 @@ static StorageMutability storageIsMutable(bool isMutable) {
 /// is a let member in an initializer.
 StorageMutability
 VarDecl::mutability(const DeclContext *UseDC,
-                    const DeclRefExpr *base) const {
+                    std::optional<const DeclRefExpr *> base) const {
   // Parameters are settable or not depending on their ownership convention.
   if (auto *PD = dyn_cast<ParamDecl>(this))
     return storageIsMutable(!PD->isImmutableInFunctionBody());
@@ -7402,9 +7404,12 @@ VarDecl::mutability(const DeclContext *UseDC,
   if (!isLet()) {
     if (hasInitAccessor()) {
       if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(UseDC)) {
-        if (base && ctor->getImplicitSelfDecl() != base->getDecl())
-          return storageIsMutable(supportsMutation());
-        return StorageMutability::Initializable;
+        // If we're referencing 'self.', it's initializable.
+        if (!base ||
+            (*base && ctor->getImplicitSelfDecl() == (*base)->getDecl()))
+          return StorageMutability::Initializable;
+
+        return storageIsMutable(supportsMutation());
       }
     }
 
@@ -7462,17 +7467,18 @@ VarDecl::mutability(const DeclContext *UseDC,
         getDeclContext()->getSelfNominalTypeDecl())
       return StorageMutability::Immutable;
 
-    if (base && CD->getImplicitSelfDecl() != base->getDecl())
-      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 StorageMutability::Immutable;
 
-    return StorageMutability::Initializable;
+    // If we were given a base and it is 'self', it's initializable.
+    if (!base || (*base && CD->getImplicitSelfDecl() == (*base)->getDecl()))
+      return StorageMutability::Initializable;
+
+    return StorageMutability::Immutable;
   }
 
   // If the 'let' has a value bound to it but has no PBD, then it is

test/decl/init/let-mutability.swift

@@ -0,0 +1,22 @@
+// RUN: %target-swift-frontend -typecheck -dump-ast %s | %FileCheck %s
+
+public struct Data {
+  init(_ bytes: [UInt8]) { }
+}
+
+internal struct Item {
+  public let data: Data
+
+  public init(tag: UInt8) {
+    self.data = Data([tag << 2])
+  }
+
+  // CHECK-LABEL: constructor_decl{{.*}}"init(tag:value:)"
+  public init(tag: UInt8, value: UInt) {
+    // CHECK: assign_expr
+    // CHECK: member_ref_expr type="@lvalue Data"
+    // CHECK-NEXT: declref_expr type="@lvalue Item"
+    // CHECK-NEXT: member_ref_expr type="Data"
+    self.data = Self(tag: tag).data
+  }
+}