Improved handling of mixed lvalues & rvalues in tuple exprs

My previous commit here didn’t work correctly for nested tuples, both
because it didn’t recurse into them to propagate access kind correctly
and because an outer TupleIndex overload (when indexing into the nested
tuple) could still be expecting an lvalue type.

This fix is much better. ConstraintSystem::resolveOverload now
correctly always expects rvalue types from rvalue tuples. And during
applyMemberRefExpr, if the overload expects an rvalue but the tuple
contains lvalues, coerceToType() correctly does any recursive munging
of the tuple expr required.

Author: gregomni

lib/Sema/CSApply.cpp

@@ -2416,35 +2416,16 @@ namespace {
 
         Type toType = simplifyType(expr->getType());
 
-        // Don't allow lvalues when indexing into a tuple expr.
-        // If we get any lvalues, add load exprs to convert the tuple to be all rvalues.
-        if (auto *tupleExpr = dyn_cast<TupleExpr>(base)) {
-          unsigned count = tupleExpr->getNumElements();
-          unsigned lvalues = 0;
-          auto &tc = cs.getTypeChecker();
-          SmallVector<TupleTypeElt, 4> tupleElts;
-          for (unsigned i = 0; i < count; i++) {
-            Expr *elementExpr = tupleExpr->getElement(i);
-            Type elementType = elementExpr->getType();
-            if (elementType->isLValueType()) {
-              lvalues++;
-              elementExpr->propagateLValueAccessKind(AccessKind::Read, true);
-              elementExpr = new (tc.Context) LoadExpr(elementExpr, elementType->getRValueType());
-              tupleExpr->setElement(i, elementExpr);
-            }
-            tupleElts.push_back(elementExpr->getType());
-          }
-          
-          if (lvalues > 0) {
-            auto &Context = tupleExpr->getType()->getASTContext();
-            tupleExpr->setType(TupleType::get(tupleElts, Context));
-            toType = toType->getRValueType();
-          }
-        }
+        // If the result type is an rvalue and the base contains lvalues, need a full
+        // tuple coercion to properly load & set access kind on all underlying elements
+        // before taking a single element.
+        baseTy = base->getType();
+        if (!toType->isLValueType() && baseTy->isLValueType())
+          base = coerceToType(base, baseTy->getRValueType(), cs.getConstraintLocator(base));
 
         return new (cs.getASTContext()) TupleElementExpr(base, dotLoc,
-                                          selected.choice.getTupleIndex(),
-                                          nameLoc.getBaseNameLoc(), toType);
+                                                         selected.choice.getTupleIndex(),
+                                                         nameLoc.getBaseNameLoc(), toType);
       }
 
       case OverloadChoiceKind::BaseType: {
@@ -4834,6 +4815,9 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
     }
 
     case ConversionRestrictionKind::LValueToRValue: {
+      if (toType->is<TupleType>() || fromType->is<TupleType>())
+        break;
+      
       // Load from the lvalue.
       expr->propagateLValueAccessKind(AccessKind::Read);
       expr = new (tc.Context) LoadExpr(expr, fromType->getRValueType());

lib/Sema/ConstraintSystem.cpp

@@ -1385,7 +1385,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
     } else {
       // When the base is a tuple rvalue, the member is always an rvalue.
       auto tuple = choice.getBaseType()->castTo<TupleType>();
-      refType = tuple->getElementType(choice.getTupleIndex());
+      refType = tuple->getElementType(choice.getTupleIndex())->getRValueType();
     }
     break;
   }

test/expr/expressions.swift

@@ -842,3 +842,5 @@ let _ = (x, 3).1
 (x,y) = (2,3)
 (x,4) = (1,2) // expected-error {{cannot assign to value: function call returns immutable value}}
 (x,y).1 = 7 // expected-error {{cannot assign to immutable expression of type 'Int'}}
+x = (x,(3,y)).1.1
+