Diagnose inout uses of 'lets' in constructors in the type checker.

Stored `let` properties of a struct, class, or actor permit
'inout' modification within the constructor body after they have been
initialized. Tentatively remove this rule, only allowing such `let`
properties to be initialized (assigned to) and not treated as `inout`.

Fixes rdar://127258363.

Author: DougGregor

include/swift/Sema/ConstraintSystem.h

@@ -2157,6 +2157,11 @@ class ConstraintSystem {
   /// Note: this is only used to support ObjCSelectorExpr at the moment.
   llvm::SmallPtrSet<Expr *, 2> UnevaluatedRootExprs;
 
+  /// Expressions that are in the left-hand side of an assignment
+  /// expression. This is used to help distinguish assignments from
+  /// any other potential use that might need lvalues.
+  llvm::SmallPtrSet<Expr *, 2> TargetOfAssignExprs;
+
   /// The total number of disjunctions created.
   unsigned CountDisjunctions = 0;
 
@@ -4436,7 +4441,10 @@ class ConstraintSystem {
       llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency =
         [](const ClosureExpr *closure) {
           return closure->isIsolatedByPreconcurrency();
-        });
+        },
+      llvm::function_ref<bool(Expr *)> isAssignTarget = [](Expr *) {
+        return true;
+      });
 
   /// Given the opened type and a pile of information about a member reference,
   /// determine the reference type of the member reference.

lib/SILGen/SILGenConcurrency.cpp

@@ -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,

lib/SILGen/SILGenConstructor.cpp

@@ -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);
 

lib/SILGen/SILGenFunction.h

@@ -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'.

lib/Sema/CSGen.cpp

@@ -4242,6 +4242,12 @@ namespace {
         return Action::SkipNode(expr);
       }
 
+      // Find all of the entries on the left-hand side of an assignment
+      // expression.
+      if (auto assign = dyn_cast<AssignExpr>(expr)) {
+        recordAssignmentTargets(assign->getDest());
+      }
+
       // Note that the subexpression of a #selector expression is
       // unevaluated.
       if (auto sel = dyn_cast<ObjCSelectorExpr>(expr)) {
@@ -4307,6 +4313,49 @@ namespace {
       return Action::Continue(expr);
     }
 
+    /// Walk the destination of an assignment expression, recording
+    /// each of the
+    void recordAssignmentTargets(Expr *dest) {
+      if (!dest)
+        return;
+
+      if (auto tuple = dyn_cast<TupleExpr>(dest)) {
+        for (auto element : tuple->getElements())
+          recordAssignmentTargets(element);
+      }
+
+      // Insert this expression. If we already walked it, there's nothing
+      // more to do.
+      auto &CS = CG.getConstraintSystem();
+      if (!CS.TargetOfAssignExprs.insert(dest).second)
+        return;
+
+      if (auto identity = dyn_cast<IdentityExpr>(dest)) {
+        recordAssignmentTargets(identity->getSubExpr());
+        return;
+      }
+
+      if (auto tryExpr = dyn_cast<AnyTryExpr>(dest)) {
+        recordAssignmentTargets(tryExpr->getSubExpr());
+        return;
+      }
+
+      if (auto lookup = dyn_cast<LookupExpr>(dest)) {
+        recordAssignmentTargets(lookup->getBase());
+        return;
+      }
+
+      if (auto dot = dyn_cast<UnresolvedDotExpr>(dest)) {
+        recordAssignmentTargets(dot->getBase());
+        return;
+      }
+
+      if (auto bind = dyn_cast<BindOptionalExpr>(dest)) {
+        recordAssignmentTargets(bind->getSubExpr());
+        return;
+      }
+    }
+
     /// Once we've visited the children of the given expression,
     /// generate constraints from the expression.
     PostWalkResult<Expr *> walkToExprPost(Expr *expr) override {

lib/Sema/ConstraintSystem.cpp

@@ -1434,9 +1434,13 @@ ConstraintSystem::getWrappedPropertyInformation(
 ///   is being accessed; must be null if and only if this is not
 ///   a type member
 static bool
-doesStorageProduceLValue(AbstractStorageDecl *storage, Type baseType,
-                         DeclContext *useDC,
-                         ConstraintLocator *memberLocator = nullptr) {
+doesStorageProduceLValue(
+    AbstractStorageDecl *storage, Type baseType,
+    DeclContext *useDC,
+    llvm::function_ref<bool(Expr *)> isAssignTarget = [](Expr *) {
+      return true;
+    },
+    ConstraintLocator *memberLocator = nullptr) {
   const DeclRefExpr *base = nullptr;
   if (memberLocator) {
     if (auto *const E = getAsExpr(memberLocator->getAnchor())) {
@@ -1448,9 +1452,33 @@ doesStorageProduceLValue(AbstractStorageDecl *storage, Type baseType,
     }
   }
 
-  // Unsettable storage decls always produce rvalues.
-  if (!storage->isSettableInSwift(useDC, base))
-    return false;
+  switch (storage->mutabilityInSwift(useDC, base)) {
+    case StorageMutability::Immutable:
+      // Immutable storage decls always produce rvalues.
+      return false;
+
+    case StorageMutability::Mutable:
+      break;
+
+    case StorageMutability::Initializable: {
+      // If the member is not known to be on the left-hand side of
+      // an assignment, treat it as an rvalue so we can't pass it
+      // inout.
+      if (!memberLocator)
+        break;
+
+      auto *anchor = getAsExpr(memberLocator->getAnchor());
+      if (!anchor)
+        break;
+
+      // If the anchor isn't known to be the target of an assignment,
+      // treat as immutable.
+      if (!isAssignTarget(anchor))
+        return false;
+
+      break;
+    }
+  }
 
   if (!storage->isSetterAccessibleFrom(useDC))
     return false;
@@ -1506,7 +1534,8 @@ Type ConstraintSystem::getUnopenedTypeOfReference(
       },
       memberLocator, wantInterfaceType, adjustForPreconcurrency,
       GetClosureType{*this},
-      ClosureIsolatedByPreconcurrency{*this});
+      ClosureIsolatedByPreconcurrency{*this},
+      [this](Expr *expr) { return TargetOfAssignExprs.contains(expr); });
 }
 
 Type ConstraintSystem::getUnopenedTypeOfReference(
@@ -1515,7 +1544,8 @@ Type ConstraintSystem::getUnopenedTypeOfReference(
     ConstraintLocator *memberLocator,
     bool wantInterfaceType, bool adjustForPreconcurrency,
     llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType,
-    llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
+    llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency,
+    llvm::function_ref<bool(Expr *)> isAssignTarget) {
   Type requestedType =
       getType(value)->getWithoutSpecifierType()->getReferenceStorageReferent();
 
@@ -1540,7 +1570,8 @@ Type ConstraintSystem::getUnopenedTypeOfReference(
 
   // Qualify storage declarations with an lvalue when appropriate.
   // Otherwise, they yield rvalues (and the access must be a load).
-  if (doesStorageProduceLValue(value, baseType, UseDC, memberLocator) &&
+  if (doesStorageProduceLValue(value, baseType, UseDC, isAssignTarget,
+                               memberLocator) &&
       !requestedType->hasError()) {
     return LValueType::get(requestedType);
   }
@@ -2711,7 +2742,11 @@ DeclReferenceType ConstraintSystem::getTypeOfMemberReference(
     if (auto *subscript = dyn_cast<SubscriptDecl>(value)) {
       auto elementTy = subscript->getElementInterfaceType();
 
-      if (doesStorageProduceLValue(subscript, baseTy, useDC, locator))
+      auto isAssignTarget = [&](Expr *expr) {
+        return TargetOfAssignExprs.contains(expr);
+      };
+      if (doesStorageProduceLValue(subscript, baseTy, useDC, isAssignTarget,
+                                   locator))
         elementTy = LValueType::get(elementTy);
 
       auto indices = subscript->getInterfaceType()

test/SILOptimizer/definite_init_diagnostics.swift

@@ -772,9 +772,9 @@ func throwingSwap<T>(_ a: inout T, _ b: inout T) throws {}
 
 // <rdar://problem/19035287> let properties should only be initializable, not reassignable
 struct LetProperties {
-  // expected-note @+1 5 {{change 'let' to 'var' to make it mutable}} {{3-6=var}}
+  // expected-note @+1 2 {{change 'let' to 'var' to make it mutable}} {{3-6=var}}
   let arr : [Int]
-  // expected-note @+1 7 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
+  // expected-note @+1 2 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
   let (u, v) : (Int, Int)
   // expected-note @+1 2 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
   let w : (Int, Int)
@@ -828,21 +828,10 @@ struct LetProperties {
   init() throws {
     u = 1; v = 13; w = (1,2); y = 1 ; z = u
 
-    var variable = 42
-    swap(&u, &variable)  // expected-error {{immutable value 'self.u' must not be passed inout}}
-    try throwingSwap(&u, &variable)  // expected-error {{immutable value 'self.u' must not be passed inout}}
-
     u.inspect()  // ok, non mutating.
-    u.mutate()  // expected-error {{mutating method 'mutate' may not be used on immutable value 'self.u'}}
 
     arr = []
-    arr += []      // expected-error {{mutating operator '+=' may not be used on immutable value 'self.arr'}}
-    arr.append(4)  // expected-error {{mutating method 'append' may not be used on immutable value 'self.arr'}}
     arr[12] = 17   // expected-error {{cannot mutate subscript of immutable value 'self.arr'}}
-    let _ = arr[replacing: 12, with: 17] // expected-error {{mutating accessor for subscript may not be used on immutable value 'self.arr'}}
-
-    methodTakesInOut(&u)  // expected-error {{immutable value 'self.u' must not be passed inout}}
-    try throwingMethodTakesInOut(&u)  // expected-error {{immutable value 'self.u' must not be passed inout}}
   }
 }
 
@@ -855,12 +844,11 @@ protocol TestMutabilityProtocol {
 
 class C<T : TestMutabilityProtocol> {
   let x : T
-  let y : T // expected-note {{change 'let' to 'var' to make it mutable}}
+  let y : T
 
   init(a : T) {
     x = a; y = a
     x.toIntMax()
-    y.changeToIntMax()  // expected-error {{mutating method 'changeToIntMax' may not be used on immutable value 'self.y'}}
   }
 }
 
@@ -949,14 +937,11 @@ struct StructMutatingMethodTest {
 
  // <rdar://problem/19268443> DI should reject this call to transparent function
  class TransparentFunction {
-  let x : Int  // expected-note {{change 'let' to 'var' to make it mutable}}
-  let y : Int  // expected-note {{change 'let' to 'var' to make it mutable}}
+  let x : Int
+  let y : Int
   init() {
     x = 42
-    x += 1     // expected-error {{mutating operator '+=' may not be used on immutable value 'self.x'}}
-
     y = 12
-    myTransparentFunction(&y)  // expected-error {{immutable value 'self.y' must not be passed inout}}
   }
 }
 

test/SILOptimizer/definite_init_existential_let.swift

@@ -15,7 +15,6 @@ class ImmutableP {
 
   init(field: P) {
     self.field = field
-    self.field.foo() // expected-error{{}}
     self.field.bar = 4 // expected-error{{}}
   }
 }
@@ -24,6 +23,5 @@ func immutableP(field: P) {
   let x: P // expected-note* {{}}
 
   x = field
-  x.foo() // expected-error{{}}
   x.bar = 4 // expected-error{{}}
 }

test/SILOptimizer/definite_init_inout_super_init.swift

@@ -15,7 +15,7 @@ class A : B {
 
   init() {
     self.x = 12
-    super.init(x: &x) // expected-error {{immutable value 'self.x' must not be passed inout}}
+    super.init(x: &x) // expected-error {{cannot pass immutable value as inout argument: 'x' is a 'let' constant}}
   }
 }
 
@@ -24,7 +24,7 @@ class C : B {
 
   init() {
     self.x = Klass()
-    super.init(x: &x) // expected-error {{immutable value 'self.x' must not be passed inout}}
+    super.init(x: &x) // expected-error {{cannot pass immutable value as inout argument: 'x' is a 'let' constant}}
   }
 }
 

test/Sema/immutability.swift

@@ -404,14 +404,19 @@ func takesClosure(_: (Int) -> Int) {
 
 func updateInt(_ x : inout Int) {}
 
+extension Int {
+  mutating func negateMe() { }
+}
+
 // rdar://15785677 - allow 'let' declarations in structs/classes be initialized in init()
 class LetClassMembers {
-  let a : Int       // expected-note 2 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
+  let a : Int       // expected-note 4 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
   let b : Int       // expected-note {{change 'let' to 'var' to make it mutable}} {{3-6=var}}
 
   init(arg : Int) {
     a = arg             // ok, a is mutable in init()
-    updateInt(&a)       // ok, a is mutable in init() and has been initialized
+    a.negateMe()        // expected-error{{cannot use mutating member on immutable value: 'a' is a 'let' constant}}
+    updateInt(&a)       // expected-error{{cannot pass immutable value as inout argument: 'a' is a 'let' constant}}
     b = 17              // ok, b is mutable in init()
   }
 
@@ -422,12 +427,13 @@ class LetClassMembers {
   }
 }
 struct LetStructMembers {
-  let a : Int       // expected-note 2 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
+  let a : Int       // expected-note 4 {{change 'let' to 'var' to make it mutable}} {{3-6=var}} {{3-6=var}}
   let b : Int       // expected-note {{change 'let' to 'var' to make it mutable}} {{3-6=var}}
 
   init(arg : Int) {
     a = arg             // ok, a is mutable in init()
-    updateInt(&a)       // ok, a is mutable in init() and has been initialized
+    updateInt(&a)       // expected-error {{cannot pass immutable value as inout argument: 'a' is a 'let' constant}}
+    a += 1              // expected-error {{left side of mutating operator isn't mutable: 'a' is a 'let' constant}}
     b = 17              // ok, b is mutable in init()
   }
 

test/Sema/immutability_overload_async.swift

@@ -0,0 +1,31 @@
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
+// RUN: %target-swift-frontend  -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-upcoming-feature RegionBasedIsolation
+
+// REQUIRES: concurrency
+// REQUIRES: asserts
+
+protocol Iterator {
+  associatedtype Failure: Error
+  mutating func next() async throws -> Int?
+}
+
+extension Iterator {
+  mutating func next() async throws(Failure) -> Int? {
+    nil
+  }
+}
+
+actor C: Iterator {
+  typealias Failure = any Error
+  func next() throws -> Int? { nil }
+}
+
+actor A {
+  let c = C()
+
+  init() async {
+    _ = try! await self.c.next()
+  }
+}