Merge pull request #27558 from AnthonyLatsis/lvalue-type-fixit

[NFC] Fix occasional @lvalue exposures

Author: xedin

lib/Sema/CSDiag.cpp

@@ -4569,7 +4569,7 @@ diagnoseAmbiguousMultiStatementClosure(ClosureExpr *closure) {
       auto type = getTypeOfExpressionWithoutApplying(
           resultExpr, CS.DC, decl, FreeTypeVariableBinding::UnresolvedType);
       if (type)
-        resultType = type;
+        resultType = type->getRValueType();
     }
 
     // If we found a type, presuppose it was the intended result and insert a

lib/Sema/CSDiagnostics.cpp

@@ -75,12 +75,14 @@ std::pair<Expr *, bool> FailureDiagnostic::computeAnchor() const {
   return {anchor, !resolved->getPath().empty()};
 }
 
-Type FailureDiagnostic::getType(Expr *expr) const {
-  return resolveType(CS.getType(expr));
+Type FailureDiagnostic::getType(Expr *expr, bool wantRValue) const {
+  return resolveType(CS.getType(expr), /*reconstituteSugar=*/false,
+                     wantRValue);
 }
 
-Type FailureDiagnostic::getType(const TypeLoc &loc) const {
-  return resolveType(CS.getType(loc));
+Type FailureDiagnostic::getType(const TypeLoc &loc, bool wantRValue) const {
+  return resolveType(CS.getType(loc), /*reconstituteSugar=*/false,
+                     wantRValue);
 }
 
 template <typename... ArgTypes>
@@ -248,7 +250,6 @@ FailureDiagnostic::getFunctionArgApplyInfo(ConstraintLocator *locator) const {
   // Try to resolve the function type by loading lvalues and looking through
   // optional types, which can occur for expressions like `fn?(5)`.
   auto *fnType = resolveType(rawFnType)
-                     ->getRValueType()
                      ->lookThroughAllOptionalTypes()
                      ->getAs<FunctionType>();
   if (!fnType)
@@ -1077,7 +1078,7 @@ bool MemberAccessOnOptionalBaseFailure::diagnoseAsError() {
     return false;
 
   auto *anchor = getAnchor();
-  auto baseType = getType(anchor)->getRValueType();
+  auto baseType = getType(anchor);
   bool resultIsOptional = ResultTypeIsOptional;
 
   // If we've resolved the member overload to one that returns an optional
@@ -1229,7 +1230,7 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
     }
 
     emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try,
-                   getType(anchor)->getRValueType())
+                   getType(anchor))
         .fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()},
                       "try!");
     return true;
@@ -1399,7 +1400,7 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
 
       if (resolvedOverload->Choice.getKind() ==
           OverloadChoiceKind::KeyPathDynamicMemberLookup) {
-        if (!getType(member->getBase())->hasLValueType())
+        if (!getType(member->getBase(), /*wantRValue=*/false)->hasLValueType())
           subElementDiagID =
               diag::assignment_dynamic_property_has_immutable_base;
       }
@@ -2299,7 +2300,7 @@ bool ContextualFailure::diagnoseYieldByReferenceMismatch() const {
     return false;
 
   auto *anchor = getAnchor();
-  auto exprType = getType(anchor);
+  auto exprType = getType(anchor, /*wantRValue=*/false);
   auto contextualType = getToType();
 
   if (auto exprLV = exprType->getAs<LValueType>()) {
@@ -5163,7 +5164,7 @@ bool ArgumentMismatchFailure::diagnoseAsError() {
   // let's match up its element type to the argument to see whether
   // it would be appropriate to suggest adding `&`.
   auto *argExpr = getAnchor();
-  if (getType(argExpr)->is<LValueType>()) {
+  if (getType(argExpr, /*wantRValue=*/false)->is<LValueType>()) {
     auto elementTy = paramType->getAnyPointerElementType();
     if (elementTy && argType->isEqual(elementTy)) {
       diag.fixItInsert(argExpr->getStartLoc(), "&");
@@ -5199,8 +5200,8 @@ bool ArgumentMismatchFailure::diagnoseUseOfReferenceEqualityOperator() const {
 
   auto name = *getOperatorName(binaryOp->getFn());
 
-  auto lhsType = getType(lhs)->getRValueType();
-  auto rhsType = getType(rhs)->getRValueType();
+  auto lhsType = getType(lhs);
+  auto rhsType = getType(rhs);
 
   // If both arguments where incorrect e.g. both are function types,
   // let's avoid producing a diagnostic second time, because first
@@ -5261,8 +5262,8 @@ bool ArgumentMismatchFailure::diagnosePatternMatchingMismatch() const {
   auto *lhsExpr = op->getArg()->getElement(0);
   auto *rhsExpr = op->getArg()->getElement(1);
 
-  auto lhsType = getType(lhsExpr)->getRValueType();
-  auto rhsType = getType(rhsExpr)->getRValueType();
+  auto lhsType = getType(lhsExpr);
+  auto rhsType = getType(rhsExpr);
 
   auto diagnostic =
       lhsType->is<UnresolvedType>()

lib/Sema/CSDiagnostics.h

@@ -92,15 +92,18 @@ class FailureDiagnostic {
 
   ConstraintLocator *getLocator() const { return Locator; }
 
-  Type getType(Expr *expr) const;
-  Type getType(const TypeLoc &loc) const;
+  Type getType(Expr *expr, bool wantRValue = true) const;
+  Type getType(const TypeLoc &loc, bool wantRValue = true) const;
 
   /// Resolve type variables present in the raw type, if any.
-  Type resolveType(Type rawType, bool reconstituteSugar = false) const {
+  Type resolveType(Type rawType, bool reconstituteSugar = false,
+                   bool wantRValue = true) const {
     auto resolvedType = CS.simplifyType(rawType);
-    return reconstituteSugar
-               ? resolvedType->reconstituteSugar(/*recursive*/ true)
-               : resolvedType;
+
+    if (reconstituteSugar)
+      resolvedType = resolvedType->reconstituteSugar(/*recursive*/ true);
+
+    return wantRValue ? resolvedType->getRValueType() : resolvedType;
   }
 
   /// Resolve type variables present in the raw type, using generic parameter
@@ -695,8 +698,7 @@ class ContextualFailure : public FailureDiagnostic {
                     ContextualTypePurpose purpose, Type lhs, Type rhs,
                     ConstraintLocator *locator)
       : FailureDiagnostic(root, cs, locator), CTP(purpose),
-        FromType(resolve(lhs)->getRValueType()),
-        ToType(resolve(rhs)->getRValueType()) {}
+        FromType(resolve(lhs)), ToType(resolve(rhs)) {}
 
   Type getFromType() const { return FromType; }
 

test/Constraints/diagnostics.swift

@@ -1274,7 +1274,7 @@ func rdar17170728() {
     // expected-error@-1 {{missing argument label 'into:' in call}}
     // expected-error@-2 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
     $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
-    // expected-error@-1 {{binary operator '+' cannot be applied to operands of type '@lvalue Bool' and 'Bool'}}
+    // expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
     // expected-error@-2 {{'nil' cannot be used in context expecting type 'Bool'}}
   }
 }

test/expr/closure/closures.swift

@@ -355,3 +355,9 @@ func lvalueCapture<T>(c: GenericClass<T>) {
     cc = wc!
   }
 }
+
+// Don't expose @lvalue-ness in diagnostics.
+let closure = { // expected-error {{unable to infer complex closure return type; add explicit type to disambiguate}} {{16-16= () -> Bool in }}
+  var helper = true
+  return helper
+}