[Diagnostics] Port the "extra address of agrument" diagnostic to the new

framework.

Author: hborla

lib/Sema/CSDiag.cpp

@@ -2199,19 +2199,6 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType,
           if (!exprResult)
             return nullptr;
 
-          // If the caller expected something inout, but we didn't have
-          // something of inout type, diagnose it.
-          if (auto IOE =
-                dyn_cast<InOutExpr>(exprResult->getSemanticsProvidingExpr())) {
-            if (!param.isInOut()) {
-              diagnose(exprResult->getLoc(), diag::extra_address_of,
-                       CS.getType(exprResult)->getInOutObjectType())
-                  .highlight(exprResult->getSourceRange())
-                  .fixItRemove(IOE->getStartLoc());
-              return nullptr;
-            }
-          }
-
           auto resultTy = CS.getType(exprResult);
           resultElts[inArgNo] = exprResult;
           resultEltTys[inArgNo] = {resultTy->getInOutObjectType(),
@@ -4592,13 +4579,6 @@ bool FailureDiagnosis::visitInOutExpr(InOutExpr *IOE) {
       }
     } else if (contextualType->is<InOutType>()) {
       contextualType = contextualType->getInOutObjectType();
-    } else {
-      // If the caller expected something inout, but we didn't have
-      // something of inout type, diagnose it.
-      diagnose(IOE->getLoc(), diag::extra_address_of, contextualType)
-        .highlight(IOE->getSourceRange())
-        .fixItRemove(IOE->getStartLoc());
-      return true;
     }
   }
 
@@ -6015,19 +5995,6 @@ bool FailureDiagnosis::visitTupleExpr(TupleExpr *TE) {
                                     CS.getContextualTypePurpose(), options);
     // If there was an error type checking this argument, then we're done.
     if (!exprResult) return true;
-    
-    // If the caller expected something inout, but we didn't have
-    // something of inout type, diagnose it.
-    if (auto IOE =
-          dyn_cast<InOutExpr>(exprResult->getSemanticsProvidingExpr())) {
-      if (!contextualTT->getElement(i).isInOut()) {
-        diagnose(exprResult->getLoc(), diag::extra_address_of,
-                 CS.getType(exprResult)->getInOutObjectType())
-            .highlight(exprResult->getSourceRange())
-            .fixItRemove(IOE->getStartLoc());
-        return true;
-      }
-    }
   }
 
   return false;

lib/Sema/CSDiagnostics.cpp

@@ -908,6 +908,17 @@ bool MissingAddressOfFailure::diagnoseAsError() {
   return true;
 }
 
+bool ExtraAddressOfFailure::diagnoseAsError() {
+  if (hasComplexLocator())
+    return false;
+
+  auto anchor = getAnchor();
+  emitDiagnostic(getAnchor()->getLoc(), diag::extra_address_of, getToType())
+      .highlight(anchor->getSourceRange())
+      .fixItRemove(anchor->getStartLoc());
+  return true;
+}
+
 bool MissingExplicitConversionFailure::diagnoseAsError() {
   if (hasComplexLocator())
     return false;

lib/Sema/CSDiagnostics.h

@@ -843,6 +843,16 @@ class MissingAddressOfFailure final : public ContextualFailure {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose failures when passing an inout argument to a non-inout parameter.
+class ExtraAddressOfFailure final : public ContextualFailure {
+public:
+  ExtraAddressOfFailure(Expr *expr, ConstraintSystem &cs, Type argTy,
+                        Type paramTy, ConstraintLocator *locator)
+      : ContextualFailure(expr, cs, argTy, paramTy, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose mismatches relating to tuple destructuring.
 class TupleContextualFailure final : public ContextualFailure {
 public:

lib/Sema/CSFix.cpp

@@ -114,6 +114,19 @@ AddAddressOf *AddAddressOf::create(ConstraintSystem &cs, Type argTy,
   return new (cs.getAllocator()) AddAddressOf(cs, argTy, paramTy, locator);
 }
 
+bool RemoveAddressOfArg::diagnose(Expr *root, bool asNote) const {
+  auto &cs = getConstraintSystem();
+  ExtraAddressOfFailure failure(root, cs, getFromType(), getToType(),
+                                getLocator());
+  return failure.diagnose(asNote);
+}
+
+RemoveAddressOfArg *
+RemoveAddressOfArg::create(ConstraintSystem &cs, Type argTy, Type paramTy,
+                           ConstraintLocator *locator) {
+  return new (cs.getAllocator()) RemoveAddressOfArg(cs, argTy, paramTy, locator);
+}
+
 bool TreatRValueAsLValue::diagnose(Expr *root, bool asNote) const {
   RValueTreatedAsLValueFailure failure(getConstraintSystem(), getLocator());
   return failure.diagnose(asNote);

lib/Sema/CSFix.h

@@ -545,6 +545,21 @@ class AddAddressOf final : public ContextualMismatch {
                               ConstraintLocator *locator);
 };
 
+/// Remove the \c & of an argument for a non-inout parameter.
+class RemoveAddressOfArg final : public ContextualMismatch {
+  RemoveAddressOfArg(ConstraintSystem &cs, Type argTy, Type paramTy,
+                     ConstraintLocator *locator)
+      : ContextualMismatch(cs, argTy, paramTy, locator) {}
+
+public:
+  std::string getName() const override { return "remove address of argument"; }
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static RemoveAddressOfArg *create(ConstraintSystem &cs, Type argTy,
+                                    Type paramTy, ConstraintLocator *locator);
+};
+
 /// Detect situations where two type's generic arguments must
 /// match but are not convertible e.g.
 ///

lib/Sema/CSSimplify.cpp

@@ -2430,6 +2430,23 @@ bool ConstraintSystem::repairFailures(
         conversionsOrFixes.push_back(AddAddressOf::create(
             *this, lhs, rhs, getConstraintLocator(locator)));
     }
+
+    // If the argument is inout and the parameter is not inout or a pointer,
+    // suggest removing the &.
+    if (lhs->is<InOutType>() && !rhs->is<InOutType>()) {
+      auto objectType = rhs->lookThroughAllOptionalTypes();
+      if (objectType->getAnyPointerElementType())
+        break;
+
+      auto result = matchTypes(lhs->getInOutObjectType(), rhs,
+                               ConstraintKind::ArgumentConversion,
+                               TypeMatchFlags::TMF_ApplyingFix, locator);
+
+      if (result.isSuccess())
+        conversionsOrFixes.push_back(RemoveAddressOfArg::create(*this, lhs,
+            rhs, getConstraintLocator(locator)));
+    }
+
     break;
   }
 

test/Constraints/diagnostics.swift

@@ -570,8 +570,8 @@ func r22255907_2<T>(_ x : Int, a : T, b: Int) {}
 
 func reachabilityForInternetConnection() {
   var variable: Int = 42
-  r22255907_1(&variable, b: 2.1) // expected-error {{'&' used with non-inout argument of type 'Int'}} {{15-16=}}
-  r22255907_2(1, a: &variable, b: 2.1)// expected-error {{'&' used with non-inout argument of type 'Int'}} {{21-22=}}
+  r22255907_1(&variable, b: 2) // expected-error {{'&' used with non-inout argument of type 'Int'}} {{15-16=}}
+  r22255907_2(1, a: &variable, b: 2)// expected-error {{'&' used with non-inout argument of type 'Int'}} {{21-22=}}
 }
 
 // <rdar://problem/21601687> QoI: Using "=" instead of "==" in if statement leads to incorrect error message

test/Generics/materializable_restrictions.swift

@@ -2,8 +2,8 @@
 
 func test15921520() {
     var x: Int = 0
-    func f<T>(_ x: T) {} // expected-note{{in call to function 'f'}}
-    f(&x) // expected-error{{generic parameter 'T' could not be inferred}}
+    func f<T>(_ x: T) {}
+    f(&x) // expected-error{{'&' used with non-inout argument of type 'Int'}} {{7-8=}}
 }
 
 func test20807269() {