Clean up tracking of throws sites

Author: DougGregor

include/swift/AST/CatchNode.h

@@ -19,7 +19,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/Stmt.h"
-
+#include "swift/AST/ASTNode.h"
 namespace swift {
 
 /// An AST node that represents a point where a thrown error can be caught and
@@ -45,6 +45,8 @@ class CatchNode: public llvm::PointerUnion<
   /// needs to be inferred.
   Type getExplicitCaughtType(ASTContext &ctx) const;
 
+  explicit operator ASTNode() const;
+
   friend llvm::hash_code hash_value(CatchNode catchNode) {
     using llvm::hash_value;
     return hash_value(catchNode.getOpaqueValue());

include/swift/Sema/ConstraintSystem.h

@@ -1446,6 +1446,15 @@ struct MatchCallArgumentResult {
 /// `x.y` is a potential throw site.
 struct PotentialThrowSite {
   enum Kind {
+    /// The caught type variable for this catch node, which is a stand-in
+    /// for the type that will be caught by the particular catch node.
+    ///
+    /// This is not strictly a potential throw site, but is convenient
+    /// to represent it as such so we don't need a separate mapping
+    /// from catch node -> caught type variable in the constraint solver's
+    /// state.
+    CaughtTypeVariable,
+
     /// The application of a function or subscript.
     Application,
 
@@ -3428,8 +3437,9 @@ class ConstraintSystem {
   /// Determine the caught error type for the given catch node.
   Type getCaughtErrorType(CatchNode node);
 
-  /// Infer the caught error type for this catch node, once we have all of
-  /// the potential throw sites.
+  /// Infer the caught error type for this catch node, or introduce an
+  /// appropriate type variable to describe that caught error type if
+  /// it cannot be computed yet.
   Type inferCaughtErrorType(CatchNode node);
 
   /// Return the type variable that represents the inferred thrown error

lib/AST/Decl.cpp

@@ -12088,6 +12088,18 @@ Type CatchNode::getExplicitCaughtType(ASTContext &ctx) const {
       ctx.evaluator, ExplicitCaughtTypeRequest{&ctx, *this}, Type());
 }
 
+CatchNode::operator ASTNode() const {
+  if (auto func = dyn_cast<AbstractFunctionDecl *>())
+    return func;
+  if (auto closure = dyn_cast<ClosureExpr *>())
+    return closure;
+  if (auto doCatch = dyn_cast<DoCatchStmt *>())
+    return doCatch;
+  if (auto anyTry = dyn_cast<AnyTryExpr *>())
+    return anyTry;
+  llvm_unreachable("Unhandled catch node");
+}
+
 void swift::simple_display(llvm::raw_ostream &out, CatchNode catchNode) {
   out << "catch node";
 }

lib/Sema/CSSimplify.cpp

@@ -13016,9 +13016,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyApplicableFnConstraint(
   // Local function to form an unsolved result.
   auto formUnsolved = [&](bool activate = false) {
     if (flags.contains(TMF_GenerateConstraints)) {
+      auto fixedLocator = getConstraintLocator(locator);
       auto *application = Constraint::createApplicableFunction(
           *this, type1, type2, trailingClosureMatching,
-          getConstraintLocator(locator));
+          fixedLocator);
 
       addUnsolvedConstraint(application);
       if (activate)
@@ -13048,10 +13049,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyApplicableFnConstraint(
   // is not valid for operators though, where an inout parameter does not
   // have an explicit inout argument.
   if (type1.getPointer() == desugar2) {
-    // Note that this could throw.
-    recordPotentialThrowSite(
-        PotentialThrowSite::Application, Type(desugar2), outerLocator);
-
     if (!isOperator || !hasInOut()) {
       recordMatchCallArgumentResult(
           getConstraintLocator(
@@ -13102,10 +13099,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyApplicableFnConstraint(
 
   // For a function, bind the output and convert the argument to the input.
   if (auto func2 = dyn_cast<FunctionType>(desugar2)) {
-    // Note that this could throw.
-    recordPotentialThrowSite(
-        PotentialThrowSite::Application, Type(desugar2), outerLocator);
-
     ConstraintKind subKind = (isOperator
                               ? ConstraintKind::OperatorArgumentConversion
                               : ConstraintKind::ArgumentConversion);
@@ -13848,6 +13841,9 @@ ConstraintSystem::simplifyCaughtErrorConstraint(
     TypeMatchOptions flags,
     ConstraintLocatorBuilder locator) {
   Type caughtErrorType = inferCaughtErrorType(catchNode);
+  if (caughtErrorType->isEqual(type))
+    return SolutionKind::Unsolved;
+
   addConstraint(ConstraintKind::Bind, type, caughtErrorType, locator);
   return SolutionKind::Solved;
 }
@@ -15719,6 +15715,14 @@ void ConstraintSystem::addConstraint(ConstraintKind kind, Type first,
                                      Type second,
                                      ConstraintLocatorBuilder locator,
                                      bool isFavored) {
+  // When adding a function-application constraint, make sure to introduce
+  // a potential throw site.
+  if (kind == ConstraintKind::ApplicableFunction) {
+    recordPotentialThrowSite(
+        PotentialThrowSite::Application, second,
+        getConstraintLocator(locator));
+  }
+
   switch (addConstraintImpl(kind, first, second, locator, isFavored)) {
   case SolutionKind::Error:
     // Add a failing constraint, if needed.

lib/Sema/CSSyntacticElement.cpp

@@ -374,12 +374,8 @@ static void createConjunction(ConstraintSystem &cs, DeclContext *dc,
   // of that closure, introduce a constraint to do so.
   if (locator->directlyAt<ClosureExpr>()) {
     auto *closure = castToExpr<ClosureExpr>(locator->getAnchor());
-    if (auto thrownErrorTypeVar = cs.getInferredThrownError(closure)) {
+    if (auto thrownErrorTypeVar = cs.getInferredThrownError(closure))
       referencedVars.push_back(thrownErrorTypeVar);
-      constraints.push_back(
-          Constraint::createCaughtError(cs, Type(thrownErrorTypeVar), closure,
-                                        locator, referencedVars));
-    }
   }
 
   // It's possible that there are no viable elements in the body,
@@ -1139,6 +1135,11 @@ class SyntacticElementConstraintGenerator
           visitDecl(node.get<Decl *>());
         }
       }
+
+      if (closure) {
+        auto closureType = cs.getClosureType(closure);
+      }
+      
       return;
     }
 

lib/Sema/ConstraintSystem.cpp

@@ -450,15 +450,56 @@ Type ConstraintSystem::inferCaughtErrorType(CatchNode catchNode) {
     }
   }
 
+  // The type variable that already describes the caught error type for
+  // this particular catch node, if it exists. These are lazily created
+  // when we are unable to determine the caught error type completely.
+  TypeVariableType *caughtTypeVariable = nullptr;
+  ConstraintLocator *caughtTypeVariableLocator = nullptr;
+
+  /// The complete set of type variables referenced by the throw sites.
+  SmallPtrSet<TypeVariableType *, 4> referencedTypeVariables;
+
+  /// The caught error type thus far, which starts at Never because Never
+  /// doesn't contribute to throwing anything.
   Type caughtErrorType = ctx.getNeverType();
   for (const auto &throwSite : throwSites) {
     Type type = simplifyType(throwSite.type);
-
     Type thrownErrorType;
     switch (throwSite.kind) {
+    case PotentialThrowSite::CaughtTypeVariable:
+      // The type variable hasn't been bound yet, so record that we
+      // know it exists and keep looking.
+      if (auto typeVar = type->getAs<TypeVariableType>()) {
+        caughtTypeVariable = typeVar;
+        caughtTypeVariableLocator = throwSite.locator;
+        continue;
+      }
+
+      // The caught type variable has been bound to a fixed type. Return
+      // that.
+      return type;
+
     case PotentialThrowSite::Application: {
-      auto fnType = type->castTo<AnyFunctionType>();
-      thrownErrorType = fnType->getEffectiveThrownErrorTypeOrNever();
+      auto fnType = type->getAs<AnyFunctionType>();
+      if (!fnType) {
+        // This applicable function constraint either still involves type
+        // variables or wasn't actually a function.
+        //
+        // If it still involves type variables, track those so we know
+        // when to look at the potential throw sites again. Otherwise,
+        // it's something like a metatype or a callable-as-function type,
+        // in which case there will be another Application throw site that
+        // handles the throw site.
+        if (type->hasTypeVariable()) {
+          type->getTypeVariables(referencedTypeVariables);
+        }
+
+        continue;
+      }
+
+      // Dig out the thrown error type from the function type.
+      thrownErrorType =
+          simplifyType(fnType->getEffectiveThrownErrorTypeOrNever());
       break;
     }
 
@@ -469,6 +510,13 @@ Type ConstraintSystem::inferCaughtErrorType(CatchNode catchNode) {
       break;
     }
 
+    // If the result has type variables, there's nothing useful we can
+    // do. Just record them and continue on.
+    if (thrownErrorType->hasTypeVariable()) {
+      thrownErrorType->getTypeVariables(referencedTypeVariables);
+      continue;
+    }
+
     // Perform the errorUnion() of the caught error type so far with the
     // thrown error type of this potential throw site.
     caughtErrorType = TypeChecker::errorUnion(
@@ -482,7 +530,46 @@ Type ConstraintSystem::inferCaughtErrorType(CatchNode catchNode) {
       break;
   }
 
-  return caughtErrorType;
+  // If the caught error type is 'any Error', or if there are no
+  // unresolved type variables, then we now have a fixed type for the
+  // caught error type.
+  if (caughtErrorType->isErrorExistentialType() ||
+      referencedTypeVariables.empty()) {
+    return caughtErrorType;
+  }
+
+  // We don't have sufficient information to provide a concrete caught
+  // error type, so we will use a type variable instead. If none exists
+  // yet, create one now.
+  if (!caughtTypeVariable) {
+    // Create the type variable.
+    caughtTypeVariableLocator = getConstraintLocator(ASTNode(catchNode));
+    caughtTypeVariable = createTypeVariable(caughtTypeVariableLocator, 0);
+
+    // Create a constraint stating that this is the caught error type
+    // for the given catch node. This constraint will get reactivated
+    // whenever any of the type variables it depends on change, causing
+    // us to re-evaluate the potential throw sites.
+    SmallVector<TypeVariableType *, 2> allTypeVars(
+        referencedTypeVariables.begin(), referencedTypeVariables.end());
+    auto constraint = Constraint::createCaughtError(
+        *this, Type(caughtTypeVariable), catchNode,
+        caughtTypeVariableLocator, allTypeVars);
+    addUnsolvedConstraint(constraint);
+
+    // Record this type variable in the list of potential throw sites,
+    // so we can find it again later rather than creating a new one.
+    // This is effectively using the potential throw sites as a
+    // CatchNode -> TypeVariableType * map, without requiring a separate
+    // data structure.
+    potentialThrowSites.push_back(
+      {catchNode,
+        PotentialThrowSite{PotentialThrowSite::CaughtTypeVariable,
+                           Type(caughtTypeVariable),
+                           caughtTypeVariableLocator}});
+  }
+
+  return Type(caughtTypeVariable);
 }
 
 TypeVariableType *
@@ -3984,6 +4071,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
   // If accessing this declaration could throw an error, record this as a
   // potential throw site.
   if (thrownErrorTypeOnAccess) {
+    // FIXME: We likely need to do this before overload resolution,
+    // otherwise we might establish the thrown error type too early.
     recordPotentialThrowSite(
         PotentialThrowSite::PropertyAccess, thrownErrorTypeOnAccess, locator);
   }

lib/Sema/TypeCheckConstraints.cpp

@@ -1679,18 +1679,22 @@ void ConstraintSystem::print(raw_ostream &out) const {
     out.indent(indent) << "Potential throw sites:\n";
     interleave(potentialThrowSites, [&](const auto &throwSite) {
       out.indent(indent + 2);
+      out << " -" << throwSite.second.type->getString(PO) << " is ";
       switch (throwSite.second.kind) {
+      case PotentialThrowSite::CaughtTypeVariable:
+        out << "caught type variable @ ";
+        break;
       case PotentialThrowSite::Application:
-        out << "- application @ ";
+        out << "application @ ";
         break;
       case PotentialThrowSite::ExplicitThrow:
-        out << " - explicit throw @ ";
+        out << "explicit throw @ ";
         break;
       case PotentialThrowSite::NonExhaustiveDoCatch:
-        out << " - non-exhaustive do..catch @ ";
+        out << "non-exhaustive do..catch @ ";
         break;
       case PotentialThrowSite::PropertyAccess:
-        out << " - property access @ ";
+        out << "property access @ ";
         break;
       }
 

test/expr/closure/typed_throws_full.swift

@@ -22,15 +22,11 @@ func testSingleStatement() {
   let c2 = {
     try doSomething()
   }
-  // FIXME: Single-expression closures aren't inferring thrown error types.
-  // expected-error@+1{{invalid conversion of thrown error type 'any Error' to 'MyError'}}
   let _: () throws(MyError) -> Int = c2
 
   let c3 = {
     return try doSomething()
   }
-  // FIXME: Single-expression closures aren't inferring thrown error types.
-  // expected-error@+1{{invalid conversion of thrown error type 'any Error' to 'MyError'}}
   let _: () throws(MyError) -> Int = c3
 }