Repository: apple/swift
File: lib/Sema/BindingProducer.cpp (C++)

Code before:
      if (auto objTy = type->getOptionalObjectType()) {
        // TODO: This could be generalized in the future to cover all patterns
        // that have an intermediate type variable in subtype/conversion chain.
        //
        // Let's not perform $T? -> $T for closure result types to avoid having
        // to re-discover solutions that differ only in location of optional
        // injection. `Void` is a special case because in $T_result position
        // it has special semantics and enables T? -> Void conversions.
        //
        // The pattern with such type variables is:
        //
        // $T_body <conv/subtype> $T_result <conv/subtype> $T_contextual_result
        //
        // When $T_contextual_result is Optional<$U>, the optional injection
        // can either happen from $T_body or from $T_result (if `return`
        // expression is non-optional), if we allow  both the solver would
        // find two solutions that differ only in location of optional
        // injection.
        if (!TypeVar->getImpl().isClosureResultType() || objTy->isVoid() ||
            objTy->isTypeVariableOrMember()) {
          // If T is a type variable, only attempt this if both the
          // type variable we are trying bindings for, and the type
          // variable we will attempt to bind, both have the same
          // polarity with respect to being able to bind lvalues.
          if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
            if (TypeVar->getImpl().canBindToLValue() ==
                otherTypeVar->getImpl().canBindToLValue()) {
              addNewBinding(binding.withType(objTy));
            }
          } else {
            addNewBinding(binding.withType(objTy));
          }
        }
      }
    }

    auto srcLocator = binding.getLocator();
    if (srcLocator &&
        (srcLocator->isLastElement<LocatorPathElt::ApplyArgToParam>() ||
         srcLocator->isLastElement<LocatorPathElt::AutoclosureResult>()) &&
        !type->hasTypeVariable() && type->isKnownStdlibCollectionType()) {
      // If the type binding comes from the argument conversion, let's
      // instead of binding collection types directly, try to bind
      // using temporary type variables substituted for element
      // types, that's going to ensure that subtype relationship is
      // always preserved.
      auto *BGT = type->castTo<BoundGenericType>();
      auto dstLocator = TypeVar->getImpl().getLocator();
      auto newType =
          CS.openUnboundGenericType(BGT->getDecl(), BGT->getParent(),
                                    dstLocator, /*isTypeResolution=*/false)

Review comment (refactor):
I think this is okay for time being but we really need to remove this condition since it's a generic problem and we keep piling up the conditions here...

Code after:
      if (auto objTy = type->getOptionalObjectType()) {
        // TODO: This could be generalized in the future to cover all patterns
        // that have an intermediate type variable in subtype/conversion chain.
        //
        // Let's not perform $T? -> $T for closure result types to avoid having
        // to re-discover solutions that differ only in location of optional
        // injection. `Void` is a special case because in $T_result position
        // it has special semantics and enables T? -> Void conversions.
        //
        // The pattern with such type variables is:
        //
        // $T_body <conv/subtype> $T_result <conv/subtype> $T_contextual_result
        //
        // When $T_contextual_result is Optional<$U>, the optional injection
        // can either happen from $T_body or from $T_result (if `return`
        // expression is non-optional), if we allow  both the solver would
        // find two solutions that differ only in location of optional
        // injection.
        //
        // The same applies to the result of a SingleValueStmtExpr, we want to
        // attempt conversions as part of the join between results rather than
        // between the expression and its context.
        auto *loc = TypeVar->getImpl().getLocator();
        if (!(TypeVar->getImpl().isClosureResultType() && !objTy->isVoid() &&
              !objTy->isTypeVariableOrMember()) &&
            !loc->directlyAt<SingleValueStmtExpr>()) {
          // If T is a type variable, only attempt this if both the
          // type variable we are trying bindings for, and the type
          // variable we will attempt to bind, both have the same
          // polarity with respect to being able to bind lvalues.
          if (auto otherTypeVar = objTy->getAs<TypeVariableType>()) {
            if (TypeVar->getImpl().canBindToLValue() ==
                otherTypeVar->getImpl().canBindToLValue()) {
              addNewBinding(binding.withType(objTy));
            }
          } else {
            addNewBinding(binding.withType(objTy));
          }
        }
      }
    }

    auto srcLocator = binding.getLocator();
    if (srcLocator &&
        (srcLocator->isLastElement<LocatorPathElt::ApplyArgToParam>() ||
         srcLocator->isLastElement<LocatorPathElt::AutoclosureResult>()) &&
        !type->hasTypeVariable() && type->isKnownStdlibCollectionType()) {
      // If the type binding comes from the argument conversion, let's
      // instead of binding collection types directly, try to bind
      // using temporary type variables substituted for element
      // types, that's going to ensure that subtype relationship is