[CSFix] Delay missing unwrap locator simplification until diagnostic

Instead of keeping two locators in the fix let's store only the
original locator and simplify it later in process of emitting
a diagnostic. That helps to avoid some duplicate work as well
as makes sure that locators supplied to the diagnostic always
have an anchor.

Resolves: rdar://problem/53344815

Author: xedin

lib/Sema/CSDiagnostics.cpp

@@ -1085,6 +1085,14 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
 
   auto *anchor = getAnchor();
 
+  // If this is an unresolved member expr e.g. `.foo` its
+  // base type is going to be the same as result type minus
+  // r-value adjustment because base could be an l-value type.
+  // We want to fix both cases by only diagnose one of them,
+  // otherwise this is just going to result in a duplcate diagnostic.
+  if (getLocator()->isLastElement(ConstraintLocator::UnresolvedMember))
+    return false;
+
   if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
     anchor = assignExpr->getSrc();
 

lib/Sema/CSFix.cpp

@@ -67,17 +67,15 @@ ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type toType,
 
 bool ForceOptional::diagnose(Expr *root, bool asNote) const {
   MissingOptionalUnwrapFailure failure(root, getConstraintSystem(), BaseType,
-                                       UnwrappedType, FullLocator);
+                                       UnwrappedType, getLocator());
   return failure.diagnose(asNote);
 }
 
 ForceOptional *ForceOptional::create(ConstraintSystem &cs, Type baseType,
                                      Type unwrappedType,
                                      ConstraintLocator *locator) {
-  auto *simplifiedLocator =
-      cs.getConstraintLocator(simplifyLocatorToAnchor(cs, locator));
   return new (cs.getAllocator())
-      ForceOptional(cs, baseType, unwrappedType, simplifiedLocator, locator);
+      ForceOptional(cs, baseType, unwrappedType, locator);
 }
 
 bool UnwrapOptionalBase::diagnose(Expr *root, bool asNote) const {

lib/Sema/CSFix.h

@@ -255,14 +255,11 @@ class ForceDowncast final : public ConstraintFix {
 class ForceOptional final : public ConstraintFix {
   Type BaseType;
   Type UnwrappedType;
-  ConstraintLocator *FullLocator;
 
   ForceOptional(ConstraintSystem &cs, Type baseType, Type unwrappedType,
-                ConstraintLocator *simplifiedLocator,
-                ConstraintLocator *fullLocator)
-      : ConstraintFix(cs, FixKind::ForceOptional, simplifiedLocator),
-        BaseType(baseType), UnwrappedType(unwrappedType),
-        FullLocator(fullLocator) {
+                ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::ForceOptional, locator), BaseType(baseType),
+        UnwrappedType(unwrappedType) {
     assert(baseType && "Base type must not be null");
     assert(unwrappedType && "Unwrapped type must not be null");
   }

lib/Sema/ConstraintLocator.cpp

@@ -170,22 +170,22 @@ bool ConstraintLocator::isForKeyPathComponent() const {
   });
 }
 
-static bool isLastElement(const ConstraintLocator *locator,
-                          ConstraintLocator::PathElementKind expectedKind) {
-  auto path = locator->getPath();
+bool ConstraintLocator::isLastElement(
+    ConstraintLocator::PathElementKind expectedKind) const {
+  auto path = getPath();
   return !path.empty() && path.back().getKind() == expectedKind;
 }
 
 bool ConstraintLocator::isForGenericParameter() const {
-  return isLastElement(this, ConstraintLocator::GenericParameter);
+  return isLastElement(ConstraintLocator::GenericParameter);
 }
 
 bool ConstraintLocator::isForSequenceElementType() const {
-  return isLastElement(this, ConstraintLocator::SequenceElementType);
+  return isLastElement(ConstraintLocator::SequenceElementType);
 }
 
 bool ConstraintLocator::isForContextualType() const {
-  return isLastElement(this, ConstraintLocator::ContextualType);
+  return isLastElement(ConstraintLocator::ContextualType);
 }
 
 GenericTypeParamType *ConstraintLocator::getGenericParameter() const {

lib/Sema/ConstraintLocator.h

@@ -593,6 +593,10 @@ class ConstraintLocator : public llvm::FoldingSetNode {
   /// Determine whether this locator points to the contextual type.
   bool isForContextualType() const;
 
+  /// Check whether the last element in the path of this locator
+  /// is of a given kind.
+  bool isLastElement(ConstraintLocator::PathElementKind kind) const;
+
   /// If this locator points to generic parameter return its type.
   GenericTypeParamType *getGenericParameter() const;
 

test/Constraints/optional.swift

@@ -357,3 +357,19 @@ struct S {
     }
   }
 }
+
+// rdar://problem/53238058 - Crash in getCalleeLocator while trying to produce a diagnostic about missing optional unwrap
+//                           associated with an argument to a call
+
+func rdar_53238058() {
+  struct S {
+    init(_: Double) {}
+    init<T>(_ value: T) where T : BinaryFloatingPoint {}
+  }
+
+  func test(_ str: String) {
+    _ = S(Double(str)) // expected-error {{value of optional type 'Double?' must be unwrapped to a value of type 'Double'}}
+    // expected-note@-1 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
+    // expected-note@-2 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+  }
+}