Merge pull request #76174 from xedin/remodel-constraint-generation-for-assignment

[ConstraintSystem] Introduce `LValueObject` constraint to replace direct l-value assignments

Author: xedin

include/swift/Sema/Constraint.h

@@ -221,6 +221,8 @@ enum class ConstraintKind : char {
   /// The first type is a tuple containing a single unlabeled element that is a
   /// pack expansion. The second type is its pattern type.
   MaterializePackExpansion,
+  /// The first type is a l-value type whose object type is the second type.
+  LValueObject,
 };
 
 /// Classification of the different kinds of constraints.
@@ -691,6 +693,7 @@ class Constraint final : public llvm::ilist_node<Constraint>,
     case ConstraintKind::PackElementOf:
     case ConstraintKind::SameShape:
     case ConstraintKind::MaterializePackExpansion:
+    case ConstraintKind::LValueObject:
       return ConstraintClassification::Relational;
 
     case ConstraintKind::ValueMember:

include/swift/Sema/ConstraintSystem.h

@@ -5239,6 +5239,12 @@ class ConstraintSystem {
                                              TypeMatchOptions flags,
                                              ConstraintLocatorBuilder locator);
 
+  /// Extract the object type of the l-value type (type1) and bind it to
+  /// to type2.
+  SolutionKind simplifyLValueObjectConstraint(Type type1, Type type2,
+                                              TypeMatchOptions flags,
+                                              ConstraintLocatorBuilder locator);
+
 public: // FIXME: Public for use by static functions.
   /// Simplify a conversion constraint with a fix applied to it.
   SolutionKind simplifyFixConstraint(ConstraintFix *fix, Type type1, Type type2,

lib/Sema/CSBindings.cpp

@@ -1318,30 +1318,7 @@ bool BindingSet::favoredOverDisjunction(Constraint *disjunction) const {
     // Such type variables might be connected to closure as well
     // e.g. when result type is optional, so it makes sense to
     // open closure before attempting such disjunction.
-    if (auto *typeVar = boundType->lookThroughAllOptionalTypes()
-                            ->getAs<TypeVariableType>()) {
-      auto fixedType = CS.getFixedType(typeVar);
-      // Handles "assignment to an overloaded member" pattern where
-      // type variable that represents the destination is bound to an
-      // l-value type during constraint generation. See \c genAssignDestType.
-      //
-      // Note that in all other circumstances we won't be here if the
-      // type variable that presents the closure is connected to a
-      // disjunction because that would mark closure as "delayed".
-      if (fixedType && fixedType->is<LValueType>()) {
-        auto lvalueObjTy = fixedType->castTo<LValueType>()->getObjectType();
-        // Prefer closure only if it's not connected to the type variable
-        // that represents l-value object type of the assignment destination.
-        // Eagerly attempting closure first could result in missing some of
-        // the contextual annotations i.e. `@Sendable`.
-        if (auto *lvalueObjVar = lvalueObjTy->getAs<TypeVariableType>())
-          return !AdjacentVars.count(lvalueObjVar);
-      }
-
-      return true;
-    }
-
-    return false;
+    return boundType->lookThroughAllOptionalTypes()->is<TypeVariableType>();
   }
 
   // If this is a collection literal type, it's preferrable to bind it
@@ -1577,6 +1554,18 @@ PotentialBindings::inferFromRelational(Constraint *constraint) {
     }
   }
 
+  // Situations like `v.<member> = { ... }` where member is overloaded.
+  // We need to wait until member is resolved otherwise there is a risk
+  // of losing some of the contextual attributes important for the closure
+  // such as @Sendable and global actor.
+  if (TypeVar->getImpl().isClosureType() &&
+      kind == AllowedBindingKind::Subtypes) {
+    if (type->isTypeVariableOrMember() &&
+        constraint->getLocator()->directlyAt<AssignExpr>()) {
+      DelayedBy.push_back(constraint);
+    }
+  }
+
   if (auto *locator = TypeVar->getImpl().getLocator()) {
     // Don't allow a protocol type to get propagated from the base to the result
     // type of a chain, Result should always be a concrete type which conforms
@@ -1586,6 +1575,24 @@ PotentialBindings::inferFromRelational(Constraint *constraint) {
       return std::nullopt;
   }
 
+  if (constraint->getKind() == ConstraintKind::LValueObject) {
+    // Allow l-value type inference from its object type, but
+    // not the other way around, that would be handled by constraint
+    // simplification.
+    if (kind == AllowedBindingKind::Subtypes) {
+      if (type->isTypeVariableOrMember())
+        return std::nullopt;
+
+      type = LValueType::get(type);
+    } else {
+      // Right-hand side of the l-value object constraint can only
+      // be bound via constraint simplification when l-value type
+      // is inferred or contextually from other constraints.
+      DelayedBy.push_back(constraint);
+      return std::nullopt;
+    }
+  }
+
   // If the source of the binding is 'OptionalObject' constraint
   // and type variable is on the left-hand side, that means
   // that it _has_ to be of optional type, since the right-hand
@@ -1757,7 +1764,8 @@ void PotentialBindings::infer(Constraint *constraint) {
   case ConstraintKind::ArgumentConversion:
   case ConstraintKind::OperatorArgumentConversion:
   case ConstraintKind::OptionalObject:
-  case ConstraintKind::UnresolvedMemberChainBase: {
+  case ConstraintKind::UnresolvedMemberChainBase:
+  case ConstraintKind::LValueObject: {
     auto binding = inferFromRelational(constraint);
     if (!binding)
       break;

lib/Sema/CSGen.cpp

@@ -3510,23 +3510,10 @@ namespace {
       } else {
         auto *locator = CS.getConstraintLocator(expr);
 
-        auto isOrCanBeLValueType = [](Type type) {
-          if (auto *typeVar = type->getAs<TypeVariableType>()) {
-            return typeVar->getImpl().canBindToLValue();
-          }
-          return type->is<LValueType>();
-        };
-
         auto exprType = CS.getType(expr);
-        if (!isOrCanBeLValueType(exprType)) {
-          // Pretend that destination is an l-value type.
-          exprType = LValueType::get(exprType);
-          (void)CS.recordFix(TreatRValueAsLValue::create(CS, locator));
-        }
-
         auto *destTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape);
-        CS.addConstraint(ConstraintKind::Bind, LValueType::get(destTy),
-                         exprType, locator);
+        CS.addConstraint(ConstraintKind::LValueObject, exprType, destTy,
+                         locator);
         return destTy;
       }
     }

lib/Sema/CSSimplify.cpp

@@ -2319,6 +2319,7 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     llvm_unreachable("Bad constraint kind in matchTupleTypes()");
   }
 
@@ -2679,6 +2680,7 @@ static bool matchFunctionRepresentations(FunctionType::ExtInfo einfo1,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     return true;
   }
 
@@ -3324,6 +3326,7 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     llvm_unreachable("Not a relational constraint");
   }
 
@@ -7185,6 +7188,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
     case ConstraintKind::ExplicitGenericArguments:
     case ConstraintKind::SameShape:
     case ConstraintKind::MaterializePackExpansion:
+    case ConstraintKind::LValueObject:
       llvm_unreachable("Not a relational constraint");
     }
   }
@@ -14158,6 +14162,80 @@ ConstraintSystem::simplifyExplicitGenericArgumentsConstraint(
   return SolutionKind::Solved;
 }
 
+ConstraintSystem::SolutionKind 
+ConstraintSystem::simplifyLValueObjectConstraint(
+    Type type1, Type type2, TypeMatchOptions flags,
+    ConstraintLocatorBuilder locator) {
+  auto lvalueTy = simplifyType(type1);
+
+  auto formUnsolved = [&]() {
+    // If we're supposed to generate constraints, do so.
+    if (flags.contains(TMF_GenerateConstraints)) {
+      auto *lvalueObject =
+          Constraint::create(*this, ConstraintKind::LValueObject,
+                             type1, type2, getConstraintLocator(locator));
+
+      addUnsolvedConstraint(lvalueObject);
+      return SolutionKind::Solved;
+    }
+
+    return SolutionKind::Unsolved;
+  };
+
+  auto isOrCanBeLValueType = [](Type type) {
+    if (auto *typeVar = type->getAs<TypeVariableType>()) {
+      return typeVar->getImpl().canBindToLValue();
+    }
+    return type->is<LValueType>();
+  };
+
+  if (lvalueTy->isPlaceholder()) {
+    if (!shouldAttemptFixes())
+      return SolutionKind::Error;
+
+    recordAnyTypeVarAsPotentialHole(type2);
+    return SolutionKind::Solved;
+  }
+
+  if (!isOrCanBeLValueType(lvalueTy)) {
+    if (!shouldAttemptFixes())
+      return SolutionKind::Error;
+
+    auto assessImpact = [&]() -> unsigned {
+      // If this is a projection of a member reference
+      // let's check whether the member is unconditionally
+      // settable, if so than it's a problem with its base.
+      if (locator.directlyAt<UnresolvedDotExpr>()) {
+        auto *memberLoc = getConstraintLocator(locator.getAnchor(),
+                                               ConstraintLocator::Member);
+        if (auto selected = findSelectedOverloadFor(memberLoc)) {
+          if (auto *storage = dyn_cast_or_null<AbstractStorageDecl>(
+                  selected->choice.getDeclOrNull())) {
+            return storage->isSettable(nullptr) ? 1 : 2;
+          }
+        }
+      }
+      return 2;
+    };
+
+    if (recordFix(
+            TreatRValueAsLValue::create(*this, getConstraintLocator(locator)),
+            assessImpact()))
+      return SolutionKind::Error;
+
+    lvalueTy = LValueType::get(lvalueTy);
+  }
+
+  if (lvalueTy->isTypeVariableOrMember())
+    return formUnsolved();
+
+  // TODO: This operation deserves its own locator just like OptionalObject.
+  addConstraint(ConstraintKind::Equal,
+                lvalueTy->castTo<LValueType>()->getObjectType(), type2,
+                getConstraintLocator(locator));
+  return SolutionKind::Solved;
+}
+
 static llvm::PointerIntPair<Type, 3, unsigned>
 getBaseTypeForPointer(TypeBase *type) {
   unsigned unwrapCount = 0;
@@ -15625,6 +15703,9 @@ ConstraintSystem::addConstraintImpl(ConstraintKind kind, Type first,
     return simplifyMaterializePackExpansionConstraint(first, second, subflags,
                                                       locator);
 
+  case ConstraintKind::LValueObject:
+    return simplifyLValueObjectConstraint(first, second, subflags, locator);
+
   case ConstraintKind::ValueMember:
   case ConstraintKind::UnresolvedValueMember:
   case ConstraintKind::ValueWitness:
@@ -16220,6 +16301,11 @@ ConstraintSystem::simplifyConstraint(const Constraint &constraint) {
     return simplifyMaterializePackExpansionConstraint(
         constraint.getFirstType(), constraint.getSecondType(),
         /*flags*/ std::nullopt, constraint.getLocator());
+
+  case ConstraintKind::LValueObject:
+    return simplifyLValueObjectConstraint(
+        constraint.getFirstType(), constraint.getSecondType(),
+        /*flags*/ std::nullopt, constraint.getLocator());
   }
 
   llvm_unreachable("Unhandled ConstraintKind in switch.");

lib/Sema/Constraint.cpp

@@ -85,6 +85,7 @@ Constraint::Constraint(ConstraintKind Kind, Type First, Type Second,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     assert(!First.isNull());
     assert(!Second.isNull());
     break;
@@ -176,6 +177,7 @@ Constraint::Constraint(ConstraintKind Kind, Type First, Type Second, Type Third,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     llvm_unreachable("Wrong constructor");
 
   case ConstraintKind::KeyPath:
@@ -326,6 +328,7 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const {
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     return create(cs, getKind(), getFirstType(), getSecondType(), getLocator());
 
   case ConstraintKind::ApplicableFunction:
@@ -590,6 +593,10 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm,
     Out << " materialize pack expansion ";
     break;
 
+  case ConstraintKind::LValueObject:
+    Out << " l-value object type ";
+    break;
+
   case ConstraintKind::Disjunction:
     llvm_unreachable("disjunction handled above");
   case ConstraintKind::Conjunction:
@@ -760,6 +767,7 @@ gatherReferencedTypeVars(Constraint *constraint,
   case ConstraintKind::ExplicitGenericArguments:
   case ConstraintKind::SameShape:
   case ConstraintKind::MaterializePackExpansion:
+  case ConstraintKind::LValueObject:
     constraint->getFirstType()->getTypeVariables(typeVars);
     constraint->getSecondType()->getTypeVariables(typeVars);
     break;

test/Constraints/diagnostics.swift

@@ -647,6 +647,7 @@ func r23560128() {
   var a : (Int,Int)?
   a.0 = 42 // expected-error{{value of optional type '(Int, Int)?' must be unwrapped to refer to member '0' of wrapped base type '(Int, Int)'}}
   // expected-note@-1{{chain the optional }}
+  // expected-note@-2 {{force-unwrap using '!' }}
 }
 
 // <rdar://problem/21890157> QoI: wrong error message when accessing properties on optional structs without unwrapping
@@ -656,6 +657,7 @@ struct ExampleStruct21890157 {
 var example21890157: ExampleStruct21890157?
 example21890157.property = "confusing"  // expected-error {{value of optional type 'ExampleStruct21890157?' must be unwrapped to refer to member 'property' of wrapped base type 'ExampleStruct21890157'}}
   // expected-note@-1{{chain the optional }}
+  // expected-note@-2 {{force-unwrap using '!' }}
 
 
 struct UnaryOp {}

test/ImportResolution/import-resolution-overload.swift

@@ -46,7 +46,8 @@ scopedFunction = 42
 // FIXME: Should be an error -- a type name and a function cannot overload.
 var _ : Int = TypeNameWins(42)
 
-TypeNameWins = 42 // expected-error {{cannot assign to immutable expression of type 'Int'}}
+// FIXME: This should be an ambiguity where both candidates are mentioned as notes.
+TypeNameWins = 42 // expected-error {{cannot assign to immutable expression of type '(Int) -> Int'}}
 var _ : TypeNameWins // no-warning
 
 // rdar://problem/21739333

validation-test/Sema/SwiftUI/rdar84580119.swift

@@ -14,7 +14,7 @@ extension EnvironmentValues {
   var myHorizontalAlignment: AlignmentID? {
     get { fatalError() }
     set { self[\.MyHorizontalAlignmentEnvironmentKey.self] = newValue }
-    // expected-error@-1 {{generic parameter 'K' could not be inferred}}
+    // expected-error@-1 {{subscript 'subscript(_:)' requires that 'any AlignmentID' be a class type}}
     // expected-error@-2 {{cannot infer key path type from context; consider explicitly specifying a root type}}
   }
 }