Merge pull request #35457 from varungandhi-apple/vg-cleanup-rep-matching

[docs] Tidy and document permissible conversions based on calling conventions
swiftlang · Jan 20, 2021 · d9cd7e7 · d9cd7e7
2 parents 8d217f3 + 29430d6
commit d9cd7e7
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 47 deletions.
diff --git a/docs/DynamicCasting.md b/docs/DynamicCasting.md
@@ -299,6 +299,7 @@ Casting from a function type F1 to a function type F2 will succeed iff the follo
 * Corresponding arguments have identical types
 * The return types are identical
 * If F1 is a throwing function type, then F2 must be a throwing function type.  If F1 is not throwing, then F2 may be a throwing or non-throwing function type.
+* F1 and F2 have the same calling convention.
 
 Note that it is _not_ sufficient for argument and return types to be castable; they must actually be identical.
 

diff --git a/include/swift/AST/ExtInfo.h b/include/swift/AST/ExtInfo.h
@@ -169,6 +169,45 @@ enum class SILFunctionTypeRepresentation : uint8_t {
   Closure,
 };
 
+/// Returns true if the function with this convention doesn't carry a context.
+constexpr bool
+isThinRepresentation(FunctionTypeRepresentation rep) {
+  switch (rep) {
+    case FunctionTypeRepresentation::Swift:
+    case FunctionTypeRepresentation::Block:
+      return false;
+    case FunctionTypeRepresentation::Thin:
+    case FunctionTypeRepresentation::CFunctionPointer:
+      return true;
+  }
+  llvm_unreachable("Unhandled FunctionTypeRepresentation in switch.");
+}
+
+/// Returns true if the function with this convention doesn't carry a context.
+constexpr bool
+isThinRepresentation(SILFunctionTypeRepresentation rep) {
+  switch (rep) {
+  case SILFunctionTypeRepresentation::Thick:
+  case SILFunctionTypeRepresentation::Block:
+    return false;
+  case SILFunctionTypeRepresentation::Thin:
+  case SILFunctionTypeRepresentation::Method:
+  case SILFunctionTypeRepresentation::ObjCMethod:
+  case SILFunctionTypeRepresentation::WitnessMethod:
+  case SILFunctionTypeRepresentation::CFunctionPointer:
+  case SILFunctionTypeRepresentation::Closure:
+    return true;
+  }
+  llvm_unreachable("Unhandled SILFunctionTypeRepresentation in switch.");
+}
+
+/// Returns true if the function with this convention carries a context.
+template <typename Repr>
+constexpr bool
+isThickRepresentation(Repr repr) {
+  return !isThinRepresentation(repr);
+}
+
 constexpr SILFunctionTypeRepresentation
 convertRepresentation(FunctionTypeRepresentation rep) {
   switch (rep) {
@@ -350,19 +389,7 @@ class ASTExtInfoBuilder {
 
   /// True if the function representation carries context.
   constexpr bool hasContext() const {
-    switch (getSILRepresentation()) {
-    case SILFunctionTypeRepresentation::Thick:
-    case SILFunctionTypeRepresentation::Block:
-      return true;
-    case SILFunctionTypeRepresentation::Thin:
-    case SILFunctionTypeRepresentation::Method:
-    case SILFunctionTypeRepresentation::ObjCMethod:
-    case SILFunctionTypeRepresentation::WitnessMethod:
-    case SILFunctionTypeRepresentation::CFunctionPointer:
-    case SILFunctionTypeRepresentation::Closure:
-      return false;
-    }
-    llvm_unreachable("Unhandled SILFunctionTypeRepresentation in switch.");
+    return isThickRepresentation(getSILRepresentation());
   }
 
   // Note that we don't have setters. That is by design, use

diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -1590,6 +1590,10 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
   // C function pointers cannot capture anything from their context.
   auto captures = SGF.SGM.Types.getLoweredLocalCaptures(constant);
 
+  // Catch cases like:
+  //   func g(_ : @convention(c) () -> ()) {}
+  //   func q() { let z = 0; func r() { print(z) }; g(r); } // error
+  // (See also: [NOTE: diagnose-swift-to-c-convention-change])
   if (!captures.getCaptures().empty() ||
       captures.hasGenericParamCaptures() ||
       captures.hasDynamicSelfCapture() ||

diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -6013,6 +6013,11 @@ maybeDiagnoseUnsupportedFunctionConversion(ConstraintSystem &cs, Expr *expr,
     if (auto closure = dyn_cast<ClosureExpr>(semanticExpr))
       return;
 
+    // Diagnose cases like:
+    //   func f() { print(w) }; func g(_ : @convention(c) () -> ()) {}
+    //   let k = f; g(k) // error
+    //   func m() { let x = 0; g({ print(x) }) } // error
+    // (See also: [NOTE: diagnose-swift-to-c-convention-change])
     de.diagnose(expr->getLoc(),
                 diag::invalid_c_function_pointer_conversion_expr);
   }

diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -1535,31 +1535,29 @@ ConstraintSystem::matchTupleTypes(TupleType *tuple1, TupleType *tuple2,
   return getTypeMatchSuccess();
 }
 
-// Determine whether conversion is allowed between two function types
-// based on their representations.
+/// Check where a representation is a subtype of another.
+///
+/// The subtype relationship is defined as:
+/// 1. any representation R is a sub-type of itself.
+/// 2. a thin representation is a subtype of any other representation.
+/// 3. a thick representation is a subtype of any other thick representation.
+///
+/// For example, since `@convention(c)` is a thin representation, and
+/// `@convention(swift)` is a thick representation,
+/// `@convention(c) (A) -> B` is a sub-type of `(A) -> B`.
+///
+/// NOTE: Unlike typical subtyping relationships, this is not anti-symmetric.
+/// For example, @convention(c) and @convention(thin) are subtypes of each other
+/// but not equal.
 static bool
-isConversionAllowedBetween(FunctionTypeRepresentation rep1,
-                           FunctionTypeRepresentation rep2) {
-  auto isThin = [](FunctionTypeRepresentation rep) {
-    return rep == FunctionTypeRepresentation::CFunctionPointer ||
-        rep == FunctionTypeRepresentation::Thin;
-  };
-
-  // Allowing "thin" (c, thin) to "thin" conventions
-  if (isThin(rep1) && isThin(rep2))
-    return true;
-
-  // Allowing all to "thick" (swift, block) conventions
-  // "thin" (c, thin) to "thick" or "thick" to "thick"
-  if (rep2 == FunctionTypeRepresentation::Swift ||
-      rep2 == FunctionTypeRepresentation::Block)
-    return true;
-
-  return rep1 == rep2;
+isSubtypeOf(FunctionTypeRepresentation potentialSubRepr,
+            FunctionTypeRepresentation potentialSuperRepr) {
+  return (potentialSubRepr == potentialSuperRepr)
+       || isThinRepresentation(potentialSubRepr)
+       || isThickRepresentation(potentialSuperRepr);
 }
 
-// Returns 'false' (i.e. no error) if it is legal to match functions with the
-// corresponding function type representations and the given match kind.
+/// Returns true if `constraint rep1 rep2` is satisfied.
 static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1,
                                          FunctionTypeRepresentation rep2,
                                          ConstraintKind kind,
@@ -1569,23 +1567,37 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1,
   case ConstraintKind::BindParam:
   case ConstraintKind::BindToPointerType:
   case ConstraintKind::Equal:
-    return rep1 != rep2;
+    return rep1 == rep2;
 
   case ConstraintKind::Subtype: {
     auto last = locator.last();
     if (!(last && last->is<LocatorPathElt::FunctionArgument>()))
-      return false;
-
-    // Inverting the result because matchFunctionRepresentations
-    // returns false in conversions are allowed.
-    return !isConversionAllowedBetween(rep1, rep2);
+      return true;
+
+    return isSubtypeOf(rep1, rep2);
   }
 
-  case ConstraintKind::OpaqueUnderlyingType:
+
+  // [NOTE: diagnose-swift-to-c-convention-change]: @convention(swift) ->
+  // @convention(c) conversions are permitted only in certain cases.
+  //
+  //   var w = 3; func f() { print(w) }; func g(_ : @convention(c) () -> ()) {}
+  //   g(f); // OK
+  //   let h = f as @convention(c) () -> (); g(h) // OK
+  //   let k = f; g(k) // error
+  //   func m() { let x = 0; g({ print(x) }) } // error
+  //   func n() { let y = 0; func p() { }; g(p); } // OK
+  //   func q() { let z = 0; func r() { print(z) }; g(r); } // error
+  //
+  // Since checking for disallowed cases requires access to captures,
+  // it is simpler to defer diagnosing (to CSApply/SILGen) and return true here.
   case ConstraintKind::Conversion:
-  case ConstraintKind::BridgingConversion:
   case ConstraintKind::ArgumentConversion:
   case ConstraintKind::OperatorArgumentConversion:
+    return true;
+
+  case ConstraintKind::OpaqueUnderlyingType:
+  case ConstraintKind::BridgingConversion:
   case ConstraintKind::ApplicableFunction:
   case ConstraintKind::DynamicCallableApplicableFunction:
   case ConstraintKind::BindOverload:
@@ -1609,7 +1621,7 @@ static bool matchFunctionRepresentations(FunctionTypeRepresentation rep1,
   case ConstraintKind::OneWayEqual:
   case ConstraintKind::OneWayBindParam:
   case ConstraintKind::DefaultClosureType:
-    return false;
+    return true;
   }
 
   llvm_unreachable("Unhandled ConstraintKind in switch.");
@@ -1902,9 +1914,9 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
       return getTypeMatchFailure(locator);
   }
 
-  if (matchFunctionRepresentations(func1->getExtInfo().getRepresentation(),
-                                   func2->getExtInfo().getRepresentation(),
-                                   kind, locator)) {
+  if (!matchFunctionRepresentations(func1->getExtInfo().getRepresentation(),
+                                    func2->getExtInfo().getRepresentation(),
+                                    kind, locator)) {
     return getTypeMatchFailure(locator);
   }