Merge pull request #69869 from DougGregor/rethrows-parameter-packs

Fix effects checking for rethrowing functions involving parameter packs

Author: DougGregor

lib/Sema/TypeCheckEffects.cpp

@@ -21,6 +21,7 @@
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Effects.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/Initializer.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Pattern.h"
@@ -279,9 +280,10 @@ class AbstractFunction {
     TheClosure = closure;
   }
 
-  explicit AbstractFunction(ParamDecl *parameter)
-    : TheKind(Kind::Parameter) {
+  explicit AbstractFunction(ParamDecl *parameter, SubstitutionMap subs)
+    : TheKind(Kind::Parameter), Substitutions(subs) {
     TheParameter = parameter;
+
   }
 
   Kind getKind() const { return TheKind; }
@@ -309,6 +311,24 @@ class AbstractFunction {
     llvm_unreachable("bad kind");
   }
 
+  /// Retrieve the interface type for a parameter based on an index into the
+  /// substituted parameter type. This
+  Type getOrigParamInterfaceType(unsigned substIndex) const {
+    switch (getKind()) {
+    case Kind::Opaque:
+    case Kind::Closure:
+    case Kind::Parameter:
+      return getType()->castTo<AnyFunctionType>()->getParams()[substIndex]
+                 .getParameterType();
+
+    case Kind::Function: {
+      auto params = getParameterList(static_cast<ValueDecl *>(getFunction()));
+      auto origIndex = params->getOrigParamIndex(getSubstitutions(), substIndex);
+      return params->get(origIndex)->getInterfaceType();
+    }
+    }
+  }
+
   bool isAutoClosure() const {
     if (getKind() == Kind::Closure)
       return isa<AutoClosureExpr>(getClosure());
@@ -386,7 +406,10 @@ class AbstractFunction {
       if (auto fn = dyn_cast<AbstractFunctionDecl>(decl)) {
         return AbstractFunction(fn, DRE->getDeclRef().getSubstitutions());
       } else if (auto param = dyn_cast<ParamDecl>(decl)) {
-        return AbstractFunction(param);
+        SubstitutionMap subs;
+        if (auto genericEnv = param->getDeclContext()->getGenericEnvironmentOfContext())
+          subs = genericEnv->getForwardingSubstitutionMap();
+        return AbstractFunction(param, subs);
       }
 
     // Closures.
@@ -813,6 +836,8 @@ class Classification {
     if (!thrownError || isNeverThrownError(thrownError))
       return result;
 
+    assert(!thrownError->hasError());
+
     result.ThrowKind = conditionalKind;
     result.ThrowReason = reason;
     result.ThrownError = thrownError;
@@ -1173,22 +1198,29 @@ class ApplyClassifier {
         // because it only counts for rethrows/reasync purposes if it
         // lines up with a throws/async function parameter in the
         // original type.
-        auto *origType = fnRef.getType()->getAs<AnyFunctionType>();
-        if (!origType) {
+        Type fnInterfaceType = fnRef.getType();
+        if (!fnInterfaceType) {
           result.merge(Classification::forInvalidCode());
           return;
         }
 
         // Use the most significant result from the arguments.
-        auto params = origType->getParams();
-        if (params.size() != args->size()) {
+        auto *fnSubstType = fnInterfaceType.subst(fnRef.getSubstitutions())
+            ->getAs<AnyFunctionType>();
+        if (!fnSubstType)  {
           result.merge(Classification::forInvalidCode());
           return;
         }
 
-        for (unsigned i = 0, e = params.size(); i < e; ++i) {
+        if (fnSubstType->getParams().size() != args->size()) {
+          result.merge(Classification::forInvalidCode());
+          return;
+        }
+
+        for (unsigned i = 0, e = args->size(); i < e; ++i) {
+          Type origParamType = fnRef.getOrigParamInterfaceType(i);
           auto argClassification = classifyArgument(
-              args->getExpr(i), params[i].getParameterType(), kind);
+              args->getExpr(i), origParamType, fnRef.getSubstitutions(), kind);
 
           // Rethrows is untyped, so adjust the thrown error type.
           if (kind == EffectKind::Throws &&
@@ -1284,7 +1316,7 @@ class ApplyClassifier {
                        EffectKind kind) {
     switch (fn.getKind()) {
     case AbstractFunction::Opaque: {
-      return classifyArgumentByType(fn.getType(),
+      return classifyArgumentByType(fn.getType(), fn.getSubstitutions(),
                                     ConditionalEffectKind::Always, reason,
                                     kind);
     }
@@ -1323,8 +1355,8 @@ class ApplyClassifier {
     else // otherwise, it throws unconditionally.
       conditional = ConditionalEffectKind::Always;
 
-    return classifyArgumentByType(param->getTypeInContext(),
-                                  conditional, reason, kind);
+    return classifyArgumentByType(
+        param->getInterfaceType(), subs, conditional, reason, kind);
   }
 
   bool isLocallyDefinedInPolymorphicEffectDeclContext(DeclContext *DC,
@@ -1653,7 +1685,8 @@ class ApplyClassifier {
   }
 
   /// Classify an argument being passed to a rethrows/reasync function.
-  Classification classifyArgument(Expr *arg, Type paramType, EffectKind kind) {
+  Classification classifyArgument(
+      Expr *arg, Type paramType, SubstitutionMap subs, EffectKind kind) {
     arg = arg->getValueProvidingExpr();
 
     if (auto *defaultArg = dyn_cast<DefaultArgumentExpr>(arg)) {
@@ -1666,7 +1699,7 @@ class ApplyClassifier {
         }
       }
 
-      return classifyArgumentByType(arg->getType(),
+      return classifyArgumentByType(arg->getType(), subs,
                                     ConditionalEffectKind::Always,
                                     PotentialEffectReason::forDefaultClosure(),
                                     kind);
@@ -1691,7 +1724,7 @@ class ApplyClassifier {
     // various tuple operations.
     if (auto paramTupleType = dyn_cast<TupleType>(paramType.getPointer())) {
       if (auto tuple = dyn_cast<TupleExpr>(arg)) {
-        return classifyTupleArgument(tuple, paramTupleType, kind);
+        return classifyTupleArgument(tuple, paramTupleType, subs, kind);
       }
 
       if (paramTupleType->getNumElements() != 1) {
@@ -1700,6 +1733,7 @@ class ApplyClassifier {
         // parameter type included a throwing function type.
         return classifyArgumentByType(
                                     paramType,
+                                    subs,
                                     ConditionalEffectKind::Always,
                                     PotentialEffectReason::forClosure(arg),
                                     kind);
@@ -1744,6 +1778,7 @@ class ApplyClassifier {
   /// Classify an argument to a rethrows/reasync function that's a tuple literal.
   Classification classifyTupleArgument(TupleExpr *tuple,
                                        TupleType *paramTupleType,
+                                       SubstitutionMap subs,
                                        EffectKind kind) {
     if (paramTupleType->getNumElements() != tuple->getNumElements())
       return Classification::forInvalidCode();
@@ -1752,7 +1787,7 @@ class ApplyClassifier {
     for (unsigned i : indices(tuple->getElements())) {
       result.merge(classifyArgument(tuple->getElement(i),
                                     paramTupleType->getElementType(i),
-                                    kind));
+                                    subs, kind));
     }
     return result;
   }
@@ -1761,7 +1796,8 @@ class ApplyClassifier {
   /// a throws/async function in a way that is permitted to cause a
   /// rethrows/reasync function to throw/async.
   static Classification 
-  classifyArgumentByType(Type paramType, ConditionalEffectKind conditional,
+  classifyArgumentByType(Type paramType, SubstitutionMap subs,
+                         ConditionalEffectKind conditional,
                          PotentialEffectReason reason, EffectKind kind) {
     if (!paramType || paramType->hasError())
       return Classification::forInvalidCode();
@@ -1779,8 +1815,10 @@ class ApplyClassifier {
         return Classification();
 
       case EffectKind::Throws:
-        if (auto thrownError = fnType->getEffectiveThrownErrorType())
-          return Classification::forThrows(*thrownError, conditional, reason);
+        if (auto thrownError = fnType->getEffectiveThrownErrorType()) {
+          return Classification::forThrows(
+              thrownError->subst(subs), conditional, reason);
+        }
 
         return Classification();
       }
@@ -1792,7 +1830,7 @@ class ApplyClassifier {
 
       for (auto eltType : tuple->getElementTypes()) {
         result.merge(
-            classifyArgumentByType(eltType, conditional, reason, kind));
+            classifyArgumentByType(eltType, subs, conditional, reason, kind));
       }
 
       return result;
@@ -2914,6 +2952,14 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
 
       auto asyncKind = classification.getConditionalKind(EffectKind::Async);
       E->setNoAsync(asyncKind == ConditionalEffectKind::None);
+    } else {
+      // HACK: functions can get queued multiple times in
+      // definedFunctions, so be sure to be idempotent.
+      if (!E->isThrowsSet()) {
+        E->setThrows(ThrownErrorDestination());
+      }
+
+      E->setNoAsync(true);
     }
 
     // If current apply expression did not type-check, don't attempt

test/decl/func/rethrows.swift

@@ -659,3 +659,32 @@ protocol P1 {
 func open(p: any P1, s: any Sequence) throws {
   _ = p.test(s).map(\.id)
 }
+
+// Rethrows checking and parameter packs, oh my.
+func rethrowsWithParameterPacks<each Arg>(_ arguments: repeat each Arg, body: () throws -> Void) rethrows { }
+
+enum MyError: Error {
+case fail
+}
+
+func testRethrowsWithParameterPacks() throws {
+  try rethrowsWithParameterPacks { throw MyError.fail }
+  rethrowsWithParameterPacks { }
+
+  try rethrowsWithParameterPacks(1) { throw MyError.fail }
+  rethrowsWithParameterPacks(1) { }
+
+  try rethrowsWithParameterPacks(1, "hello") { throw MyError.fail }
+  rethrowsWithParameterPacks(1, "hello") { }
+
+  rethrowsWithParameterPacks { throw MyError.fail }
+  // expected-error@-1{{call can throw but is not marked with 'try'}}
+  // expected-note@-2{{call is to 'rethrows' function, but argument function can throw}}
+}
+
+// Rethrows checking with the original parameter type providing the cues.
+func takesArbitraryAndRethrows<T>(_ value: T, body: () throws -> Void) rethrows { }
+
+func testArbitraryAndRethrows() {
+  takesArbitraryAndRethrows(throwingFunc) { }
+}