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

This reverts commit 80d949e.

Author: DougGregor

include/swift/Sema/ConstraintSystem.h

@@ -2092,14 +2092,6 @@ 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
@@ -4412,7 +4404,7 @@ class ConstraintSystem {
   /// \param wantInterfaceType Whether we want the interface type, if available.
   Type getUnopenedTypeOfReference(VarDecl *value, Type baseType,
                                   DeclContext *UseDC,
-                                  ConstraintLocator *locator,
+                                  ConstraintLocator *memberLocator = nullptr,
                                   bool wantInterfaceType = false,
                                   bool adjustForPreconcurrency = true);
 
@@ -4434,7 +4426,7 @@ class ConstraintSystem {
   getUnopenedTypeOfReference(
       VarDecl *value, Type baseType, DeclContext *UseDC,
       llvm::function_ref<Type(VarDecl *)> getType,
-      ConstraintLocator *locator,
+      ConstraintLocator *memberLocator = nullptr,
       bool wantInterfaceType = false,
       bool adjustForPreconcurrency = true,
       llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType =
@@ -4444,10 +4436,7 @@ 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.

lib/SILGen/SILGenConcurrency.cpp

@@ -648,8 +648,7 @@ bool SILGenFunction::unsafelyInheritsExecutor() {
 
 void ExecutorBreadcrumb::emit(SILGenFunction &SGF, SILLocation loc) {
   if (mustReturnToExecutor) {
-    assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor() ||
-           SGF.isCtorWithHopsInjectedByDefiniteInit());
+    assert(SGF.ExpectedExecutor || SGF.unsafelyInheritsExecutor());
     if (auto executor = SGF.ExpectedExecutor)
       SGF.B.createHopToExecutor(
           RegularLocation::getDebugOnlyLocation(loc, SGF.getModule()), executor,

lib/SILGen/SILGenConstructor.cpp

@@ -618,19 +618,6 @@ 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,10 +820,6 @@ 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/CSApply.cpp

@@ -655,33 +655,12 @@ namespace {
       }
 
       auto ref = resolveConcreteDeclRef(decl, locator);
-
-      // If we have a variable that's treated as an rvalue but allows
-      // assignment (for initialization) in the current context,
-      // treat it as an rvalue that we immediately load. This is
-      // the AST that's expected by SILGen.
-      bool loadImmediately = false;
-      if (auto var = dyn_cast_or_null<VarDecl>(ref.getDecl())) {
-        if (!fullType->hasLValueType()) {
-          if (var->mutability(dc) == StorageMutability::Initializable) {
-            fullType = LValueType::get(fullType);
-            adjustedFullType = LValueType::get(adjustedFullType);
-            loadImmediately = true;
-          }
-        }
-      }
-
-
       auto declRefExpr =
           new (ctx) DeclRefExpr(ref, loc, implicit, semantics, fullType);
       cs.cacheType(declRefExpr);
       declRefExpr->setFunctionRefKind(choice.getFunctionRefKind());
       Expr *result = adjustTypeForDeclReference(
           declRefExpr, fullType, adjustedFullType, locator);
-      // If we have to load, do so now.
-      if (loadImmediately)
-        result = cs.addImplicitLoadExpr(result);
-
       result = forceUnwrapIfExpected(result, locator);
 
       if (auto *fnDecl = dyn_cast<AbstractFunctionDecl>(decl)) {

lib/Sema/ConstraintSystem.cpp

@@ -1426,14 +1426,12 @@ 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,
-    llvm::function_ref<bool(Expr *)> isAssignTarget,
-    ConstraintLocator *locator) {
+doesStorageProduceLValue(AbstractStorageDecl *storage, Type baseType,
+                         DeclContext *useDC,
+                         ConstraintLocator *memberLocator = nullptr) {
   const DeclRefExpr *base = nullptr;
-  if (locator) {
-    if (auto *const E = getAsExpr(locator->getAnchor())) {
+  if (memberLocator) {
+    if (auto *const E = getAsExpr(memberLocator->getAnchor())) {
       if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
         base = dyn_cast<DeclRefExpr>(MRE->getBase());
       } else if (auto *UDE = dyn_cast<UnresolvedDotExpr>(E)) {
@@ -1442,33 +1440,9 @@ doesStorageProduceLValue(
     }
   }
 
-  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 (!locator)
-        return false;
-
-      auto *anchor = getAsExpr(simplifyLocatorToAnchor(locator));
-      if (!anchor)
-        return false;
-
-      // If the anchor isn't known to be the target of an assignment,
-      // treat as immutable.
-      if (!isAssignTarget(anchor))
-        return false;
-
-      break;
-    }
-  }
+  // Unsettable storage decls always produce rvalues.
+  if (!storage->isSettableInSwift(useDC, base))
+    return false;
 
   if (!storage->isSetterAccessibleFrom(useDC))
     return false;
@@ -1508,7 +1482,7 @@ ClosureIsolatedByPreconcurrency::operator()(const ClosureExpr *expr) const {
 
 Type ConstraintSystem::getUnopenedTypeOfReference(
     VarDecl *value, Type baseType, DeclContext *UseDC,
-    ConstraintLocator *locator, bool wantInterfaceType,
+    ConstraintLocator *memberLocator, bool wantInterfaceType,
     bool adjustForPreconcurrency) {
   return ConstraintSystem::getUnopenedTypeOfReference(
       value, baseType, UseDC,
@@ -1522,20 +1496,18 @@ Type ConstraintSystem::getUnopenedTypeOfReference(
 
         return wantInterfaceType ? var->getInterfaceType() : var->getTypeInContext();
       },
-      locator, wantInterfaceType, adjustForPreconcurrency,
+      memberLocator, wantInterfaceType, adjustForPreconcurrency,
       GetClosureType{*this},
-      ClosureIsolatedByPreconcurrency{*this},
-      IsInLeftHandSideOfAssignment{*this});
+      ClosureIsolatedByPreconcurrency{*this});
 }
 
 Type ConstraintSystem::getUnopenedTypeOfReference(
     VarDecl *value, Type baseType, DeclContext *UseDC,
     llvm::function_ref<Type(VarDecl *)> getType,
-    ConstraintLocator *locator,
+    ConstraintLocator *memberLocator,
     bool wantInterfaceType, bool adjustForPreconcurrency,
     llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType,
-    llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency,
-    llvm::function_ref<bool(Expr *)> isAssignTarget) {
+    llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
   Type requestedType =
       getType(value)->getWithoutSpecifierType()->getReferenceStorageReferent();
 
@@ -1560,8 +1532,7 @@ 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, isAssignTarget,
-                               locator) &&
+  if (doesStorageProduceLValue(value, baseType, UseDC, memberLocator) &&
       !requestedType->hasError()) {
     return LValueType::get(requestedType);
   }
@@ -1934,8 +1905,7 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value,
   // FIXME: @preconcurrency
   bool wantInterfaceType = !varDecl->getDeclContext()->isLocalContext();
   Type valueType =
-      getUnopenedTypeOfReference(varDecl, Type(), useDC, 
-                                 getConstraintLocator(locator),
+      getUnopenedTypeOfReference(varDecl, Type(), useDC, /*base=*/nullptr,
                                  wantInterfaceType);
 
   Type thrownErrorType;
@@ -2609,48 +2579,6 @@ bool ConstraintSystem::isPartialApplication(ConstraintLocator *locator) {
   return level < (baseTy->is<MetatypeType>() ? 1 : 2);
 }
 
-bool IsInLeftHandSideOfAssignment::operator()(Expr *expr) const {
-  // Walk up the parent tree.
-  auto parent = cs.getParentExpr(expr);
-  if (!parent)
-    return false;
-
-  // If the parent is an assignment expression, check whether we are
-  // the left-hand side.
-  if (auto assignParent = dyn_cast<AssignExpr>(parent)) {
-    return expr == assignParent->getDest();
-  }
-
-  // Always look up through these parent kinds.
-  if (isa<TupleExpr>(parent) || isa<IdentityExpr>(parent) ||
-      isa<AnyTryExpr>(parent) || isa<BindOptionalExpr>(parent)) {
-    return (*this)(parent);
-  }
-
-  // If the parent is a lookup expression, only follow from the base.
-  if (auto lookupParent = dyn_cast<LookupExpr>(parent)) {
-    if (lookupParent->getBase() == expr)
-      return (*this)(parent);
-
-    // The expression wasn't in the base, so this isn't part of the
-    // left-hand side.
-    return false;
-  }
-
-  // If the parent is an unresolved member reference a.b, only follow
-  // from the base.
-  if (auto dotParent = dyn_cast<UnresolvedDotExpr>(parent)) {
-    if (dotParent->getBase() == expr)
-      return (*this)(parent);
-
-    // The expression wasn't in the base, so this isn't part of the
-    // left-hand side.
-    return false;
-  }
-
-  return false;
-}
-
 DeclReferenceType
 ConstraintSystem::getTypeOfMemberReference(
     Type baseTy, ValueDecl *value, DeclContext *useDC,
@@ -2756,9 +2684,7 @@ ConstraintSystem::getTypeOfMemberReference(
     if (auto *subscript = dyn_cast<SubscriptDecl>(value)) {
       auto elementTy = subscript->getElementInterfaceType();
 
-      if (doesStorageProduceLValue(
-              subscript, baseTy, useDC,
-              IsInLeftHandSideOfAssignment{*this}, locator))
+      if (doesStorageProduceLValue(subscript, baseTy, useDC, locator))
         elementTy = LValueType::get(elementTy);
 
       auto indices = subscript->getInterfaceType()
@@ -3013,10 +2939,7 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator,
     if (auto subscript = dyn_cast<SubscriptDecl>(decl)) {
       auto elementTy = subscript->getElementInterfaceType();
 
-      if (doesStorageProduceLValue(subscript, overload.getBaseType(),
-                                   useDC,
-                                   IsInLeftHandSideOfAssignment{*this},
-                                   locator))
+      if (doesStorageProduceLValue(subscript, overload.getBaseType(), useDC))
         elementTy = LValueType::get(elementTy);
       else if (elementTy->hasDynamicSelfType()) {
         elementTy = withDynamicSelfResultReplaced(elementTy,
@@ -3040,9 +2963,7 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator,
           locator);
     } else if (auto var = dyn_cast<VarDecl>(decl)) {
       type = var->getValueInterfaceType();
-      if (doesStorageProduceLValue(
-              var, overload.getBaseType(), useDC,
-              IsInLeftHandSideOfAssignment{*this}, locator)) {
+      if (doesStorageProduceLValue(var, overload.getBaseType(), useDC)) {
         type = LValueType::get(type);
       } else if (type->hasDynamicSelfType()) {
         type = withDynamicSelfResultReplaced(type, /*uncurryLevel=*/0);

lib/Sema/TypeCheckExpr.cpp

@@ -581,6 +581,17 @@ bool TypeChecker::requireArrayLiteralIntrinsics(ASTContext &ctx,
   return true;
 }
 
+Expr *TypeChecker::buildCheckedRefExpr(VarDecl *value, DeclContext *UseDC,
+                                       DeclNameLoc loc, bool Implicit) {
+  auto type = constraints::ConstraintSystem::getUnopenedTypeOfReference(
+      value, Type(), UseDC,
+      [&](VarDecl *var) -> Type { return value->getTypeInContext(); });
+  auto semantics = value->getAccessSemanticsFromContext(UseDC,
+                                                       /*isAccessOnSelf*/false);
+  return new (value->getASTContext())
+      DeclRefExpr(value, loc, Implicit, semantics, type);
+}
+
 Expr *TypeChecker::buildRefExpr(ArrayRef<ValueDecl *> Decls,
                                 DeclContext *UseDC, DeclNameLoc NameLoc,
                                 bool Implicit, FunctionRefKind functionRefKind) {

lib/Sema/TypeChecker.h

@@ -944,6 +944,10 @@ Comparison compareDeclarations(DeclContext *dc, ValueDecl *decl1,
 /// of the first one and the expression will still type check.
 bool isDeclRefinementOf(ValueDecl *declA, ValueDecl *declB);
 
+/// Build a type-checked reference to the given value.
+Expr *buildCheckedRefExpr(VarDecl *D, DeclContext *UseDC, DeclNameLoc nameLoc,
+                          bool Implicit);
+
 /// Build a reference to a declaration, where name lookup returned
 /// the given set of declarations.
 Expr *buildRefExpr(ArrayRef<ValueDecl *> Decls, DeclContext *UseDC,

test/Constraints/diagnostics.swift

@@ -1396,9 +1396,7 @@ do {
   func generic<T>(_ value: inout T, _ closure: (S<T>) -> Void) {}
 
   let arg: Int
-  // expected-note@-1{{change 'let' to 'var' to make it mutable}}
   generic(&arg) { (g: S<Double>) -> Void in } // expected-error {{cannot convert value of type '(S<Double>) -> Void' to expected argument type '(S<Int>) -> Void'}}
-  // expected-error@-1{{cannot pass immutable value as inout argument: 'arg' is a 'let' constant}}
 }
 
 // rdar://problem/62428353 - bad error message for passing `T` where `inout T` was expected
@@ -1558,17 +1556,18 @@ func testNilCoalescingOperatorRemoveFix() {
   let _ = "" /* This is a comment */ ?? "" // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{13-43=}}
 
   let _ = "" // This is a comment
-    ?? "" // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1560:13-1561:10=}}
+    ?? "" // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1558:13-1559:10=}}
 
   let _ = "" // This is a comment
     /*
      * The blank line below is part of the test case, do not delete it
      */
-    ?? "" // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1563:13-1567:10=}}
 
-  if ("" ?? // This is a comment // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{9-1570:9=}}
+    ?? "" // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1561:13-1566:10=}}
+
+  if ("" ?? // This is a comment // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{9-1569:9=}}
       "").isEmpty {}
 
   if ("" // This is a comment
-      ?? "").isEmpty {} // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1572:9-1573:12=}}
+      ?? "").isEmpty {} // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'String', so the right side is never used}} {{1571:9-1572:12=}}
 }