[ConstraintSystem] Detect and diagnose incorrectly typed weak declarations

All `weak` declarations are supposed to have an optional type.
Detect situations when is not an optional, wrap it as a fix,
and diagnose the problem.

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -7784,7 +7784,7 @@ bool InvalidWeakAttributeUse::diagnoseAsError() {
     return false;
 
   auto *var = pattern->getDecl();
-  auto varType = getType(var);
+  auto varType = OptionalType::get(getType(var));
 
   auto diagnostic =
       emitDiagnosticAt(var, diag::invalid_ownership_not_optional,

lib/Sema/CSGen.cpp

@@ -2210,31 +2210,19 @@ namespace {
           oneWayVarType = CS.createTypeVariable(
               CS.getConstraintLocator(locator), TVO_CanBindToNoEscape);
 
-          // If there is an externally-imposed pattern type and the
-          // binding/capture is marked as `weak`, let's make sure
-          // that the imposed type is optional.
+          // If there is externally-imposed type, and the variable
+          // is marked as `weak`, let's fallthrough and allow the
+          // `one-way` constraint to be fixed in diagnostic mode.
           //
-          // Note that there is no need to check `varType` since
-          // it's only "externally" bound if this pattern isn't marked
-          // as `weak`.
-          if (externalPatternType &&
-              optionality == ReferenceOwnershipOptionality::Required) {
-            // If the type is not yet known, let's add a constraint
-            // to make sure that it can only be bound to an optional type.
-            if (externalPatternType->isTypeVariableOrMember()) {
-              auto objectTy = CS.createTypeVariable(
-                  CS.getConstraintLocator(locator.withPathElement(
-                      ConstraintLocator::OptionalPayload)),
-                  TVO_CanBindToLValue | TVO_CanBindToNoEscape);
-
-              CS.addConstraint(ConstraintKind::OptionalObject,
-                               externalPatternType, objectTy, locator);
-            } else if (!externalPatternType->getOptionalObjectType()) {
-              // TODO(diagnostics): A tailored fix to indiciate that `weak`
-              // should have an optional type.
-              return Type();
-            }
-          }
+          // That would make sure that type of this variable is
+          // recorded in the constraint  system, which would then
+          // be used instead of `getVarType` upon discovering a
+          // reference to this variable in subsequent expression(s).
+          //
+          // If we let constraint generation fail here, it would trigger
+          // interface type request via `var->getType()` that would
+          // attempt to validate `weak` attribute, and produce a
+          // diagnostic in the middle of the solver path.
 
           CS.addConstraint(ConstraintKind::OneWayEqual, oneWayVarType,
                            externalPatternType ? externalPatternType : varType,

lib/Sema/CSSimplify.cpp

@@ -4956,6 +4956,25 @@ bool ConstraintSystem::repairFailures(
           *this, lhs, rhs, getConstraintLocator(locator)));
     }
 
+    // `weak` declaration with an explicit non-optional type e.g.
+    // `weak x: X = ...` where `X` is a class.
+    if (auto *TP = dyn_cast<TypedPattern>(pattern)) {
+      if (auto *NP = dyn_cast<NamedPattern>(TP->getSubPattern())) {
+        auto *var = NP->getDecl();
+
+        auto ROK = ReferenceOwnership::Strong;
+        if (auto *OA = var->getAttrs().getAttribute<ReferenceOwnershipAttr>())
+          ROK = OA->get();
+
+        if (!rhs->getOptionalObjectType() &&
+            optionalityOf(ROK) == ReferenceOwnershipOptionality::Required) {
+          conversionsOrFixes.push_back(
+              AllowNonOptionalWeak::create(*this, getConstraintLocator(NP)));
+          break;
+        }
+      }
+    }
+
     break;
   }
 
@@ -11544,8 +11563,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowInvalidStaticMemberRefOnProtocolMetatype:
   case FixKind::AllowWrappedValueMismatch:
   case FixKind::RemoveExtraneousArguments:
-  case FixKind::SpecifyTypeForPlaceholder:
-  case FixKind::AllowNonOptionalWeak: {
+  case FixKind::SpecifyTypeForPlaceholder: {
     return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
   }
 
@@ -11710,6 +11728,17 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     return SolutionKind::Solved;
   }
 
+  case FixKind::AllowNonOptionalWeak: {
+    if (recordFix(fix))
+      return SolutionKind::Error;
+
+    (void)matchTypes(type1, OptionalType::get(type2),
+                     ConstraintKind::Conversion,
+                     TypeMatchFlags::TMF_ApplyingFix, locator);
+
+    return SolutionKind::Solved;
+  }
+
   case FixKind::UseSubscriptOperator:
   case FixKind::ExplicitlyEscaping:
   case FixKind::MarkGlobalActorFunction:

test/Constraints/result_builder_diags.swift

@@ -781,3 +781,19 @@ func test_rdar65667992() {
     }
   }
 }
+
+func test_weak_with_nonoptional_type() {
+  class X {
+    func test() -> Int { 0 }
+  }
+
+  tuplify(true) { c in
+    weak var x: X = X() // expected-error {{'weak' variable should have optional type 'X?'}}
+
+    if let x = x {
+      x.test()
+    }
+
+    42
+  }
+}