Revert "Revert "[SILGen] Fix the type of closure thunks that are passed const reference structs (#76903)" (#77309)"

This reverts commit f73c2e5.

Author: Gabor Horvath

include/swift/AST/Expr.h

@@ -29,6 +29,7 @@
 #include "swift/AST/FunctionRefInfo.h"
 #include "swift/AST/ProtocolConformanceRef.h"
 #include "swift/AST/ThrownErrorDestination.h"
+#include "swift/AST/Type.h"
 #include "swift/AST/TypeAlignments.h"
 #include "swift/Basic/Debug.h"
 #include "swift/Basic/InlineBitfield.h"
@@ -3367,9 +3368,8 @@ class UnresolvedTypeConversionExpr : public ImplicitConversionExpr {
 /// FIXME: This should be a CapturingExpr.
 class FunctionConversionExpr : public ImplicitConversionExpr {
 public:
-  FunctionConversionExpr(Expr *subExpr, Type type)
-    : ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {}
-  
+  FunctionConversionExpr(Expr *subExpr, Type type);
+
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::FunctionConversion;
   }
@@ -4290,6 +4290,12 @@ class ClosureExpr : public AbstractClosureExpr {
   /// The body of the closure.
   BraceStmt *Body;
 
+  /// Used when lowering ClosureExprs to C function pointers.
+  /// This is required to access the ClangType from SILDeclRef.
+  /// TODO: this will be redundant after we preserve ClangTypes
+  /// in the canonical types.
+  FunctionConversionExpr *ConvertedTo;
+
   friend class GlobalActorAttributeRequest;
 
   bool hasNoGlobalActorAttribute() const {
@@ -4301,19 +4307,19 @@ class ClosureExpr : public AbstractClosureExpr {
   }
 
 public:
-  ClosureExpr(const DeclAttributes &attributes,
-              SourceRange bracketRange, VarDecl *capturedSelfDecl,
-              ParameterList *params, SourceLoc asyncLoc, SourceLoc throwsLoc,
-              TypeExpr *thrownType, SourceLoc arrowLoc, SourceLoc inLoc,
-              TypeExpr *explicitResultType, DeclContext *parent)
-    : AbstractClosureExpr(ExprKind::Closure, Type(), /*Implicit=*/false,
-                          parent),
-      Attributes(attributes), BracketRange(bracketRange),
-      CapturedSelfDecl(capturedSelfDecl),
-      AsyncLoc(asyncLoc), ThrowsLoc(throwsLoc), ArrowLoc(arrowLoc),
-      InLoc(inLoc), ThrownType(thrownType),
-      ExplicitResultTypeAndBodyState(explicitResultType, BodyState::Parsed),
-      Body(nullptr) {
+  ClosureExpr(const DeclAttributes &attributes, SourceRange bracketRange,
+              VarDecl *capturedSelfDecl, ParameterList *params,
+              SourceLoc asyncLoc, SourceLoc throwsLoc, TypeExpr *thrownType,
+              SourceLoc arrowLoc, SourceLoc inLoc, TypeExpr *explicitResultType,
+              DeclContext *parent)
+      : AbstractClosureExpr(ExprKind::Closure, Type(), /*Implicit=*/false,
+                            parent),
+        Attributes(attributes), BracketRange(bracketRange),
+        CapturedSelfDecl(capturedSelfDecl), AsyncLoc(asyncLoc),
+        ThrowsLoc(throwsLoc), ArrowLoc(arrowLoc), InLoc(inLoc),
+        ThrownType(thrownType),
+        ExplicitResultTypeAndBodyState(explicitResultType, BodyState::Parsed),
+        Body(nullptr), ConvertedTo(nullptr) {
     setParameterList(params);
     Bits.ClosureExpr.HasAnonymousClosureVars = false;
     Bits.ClosureExpr.ImplicitSelfCapture = false;
@@ -4528,6 +4534,9 @@ class ClosureExpr : public AbstractClosureExpr {
     ExplicitResultTypeAndBodyState.setInt(v);
   }
 
+  const FunctionConversionExpr *getConvertedTo() const { return ConvertedTo; }
+  void setConvertedTo(FunctionConversionExpr *e) { ConvertedTo = e; }
+
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::Closure;
   }

include/swift/SIL/SILDeclRef.h

@@ -34,6 +34,10 @@ namespace llvm {
   class raw_ostream;
 }
 
+namespace clang {
+class Type;
+}
+
 namespace swift {
   enum class EffectsKind : uint8_t;
   class AbstractFunctionDecl;
@@ -261,11 +265,9 @@ struct SILDeclRef {
   ///   for the containing ClassDecl.
   /// - If 'loc' is a global VarDecl, this returns its GlobalAccessor
   ///   SILDeclRef.
-  explicit SILDeclRef(
-      Loc loc,
-      bool isForeign = false,
-      bool isDistributed = false,
-      bool isDistributedLocal = false);
+  explicit SILDeclRef(Loc loc, bool isForeign = false,
+                      bool isDistributed = false,
+                      bool isDistributedLocal = false);
 
   /// See above put produces a prespecialization according to the signature.
   explicit SILDeclRef(Loc loc, GenericSignature prespecializationSig);

lib/AST/Expr.cpp

@@ -1456,6 +1456,13 @@ DestructureTupleExpr::create(ASTContext &ctx,
                                          srcExpr, dstExpr, ty);
 }
 
+FunctionConversionExpr::FunctionConversionExpr(Expr *subExpr, Type type)
+    : ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {
+  if (auto *CE = dyn_cast<ClosureExpr>(subExpr)) {
+    CE->setConvertedTo(this);
+  }
+}
+
 SourceRange TupleExpr::getSourceRange() const {
   auto start = LParenLoc;
   if (start.isInvalid()) {

lib/ClangImporter/Serializability.cpp

@@ -302,19 +302,25 @@ namespace {
     void writeUInt64(uint64_t value) {}
     void writeIdentifier(const clang::IdentifierInfo *ident) {}
     void writeStmtRef(const clang::Stmt *stmt) {
-      if (stmt != nullptr)
+      if (stmt != nullptr) {
         IsSerializable = false;
+        llvm::errs() << "Foo\n";
+      }
     }
     void writeDeclRef(const clang::Decl *decl) {
-      if (decl && !Impl.findStableSerializationPath(decl))
+      if (decl && !Impl.findStableSerializationPath(decl)) {
         IsSerializable = false;
+        llvm::errs() << "Foo\n";
+      }
     }
     void writeSourceLocation(clang::SourceLocation loc) {
       // If a source location is written into a type, it's likely to be
       // something like the location of a VLA which we shouldn't simply
       // replace with a meaningless location.
-      if (loc.isValid())
+      if (loc.isValid()) {
         IsSerializable = false;
+        llvm::errs() << "Foo\n";
+      }
     }
 
     void writeAttr(const clang::Attr *attr) {}

lib/SIL/IR/SILFunctionType.cpp

@@ -16,6 +16,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "swift/AST/Expr.h"
+#include "swift/AST/Type.h"
 #define DEBUG_TYPE "libsil"
 
 #include "swift/AST/AnyFunctionRef.h"
@@ -4321,12 +4323,10 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
   // The type of the native-to-foreign thunk for a swift closure.
   if (constant.isForeign && constant.hasClosureExpr() &&
       shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
-    auto clangType = TC.Context.getClangFunctionType(
-        origLoweredInterfaceType->getParams(),
-        origLoweredInterfaceType->getResult(),
-        FunctionTypeRepresentation::CFunctionPointer);
-    AbstractionPattern pattern =
-        AbstractionPattern(origLoweredInterfaceType, clangType);
+    assert(!extInfoBuilder.getClangTypeInfo().empty() &&
+           "clang type not found");
+    AbstractionPattern pattern = AbstractionPattern(
+        origLoweredInterfaceType, extInfoBuilder.getClangTypeInfo().getType());
     return getSILFunctionTypeForAbstractCFunction(
         TC, pattern, origLoweredInterfaceType, extInfoBuilder, constant);
   }
@@ -4834,9 +4834,22 @@ getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
   if (!constant.isForeign)
     return AbstractionPattern(fnType);
 
+  if (const auto *closure = dyn_cast_or_null<ClosureExpr>(
+          constant.loc.dyn_cast<AbstractClosureExpr *>())) {
+    if (const auto *convertedTo = closure->getConvertedTo()) {
+      auto clangInfo = convertedTo->getType()
+                           ->castTo<AnyFunctionType>()
+                           ->getExtInfo()
+                           .getClangTypeInfo();
+      if (!clangInfo.empty())
+        return AbstractionPattern(fnType, clangInfo.getType());
+    }
+  }
+
   auto bridgedFn = getBridgedFunction(constant);
   if (!bridgedFn)
     return AbstractionPattern(fnType);
+
   const clang::Decl *clangDecl = bridgedFn->getClangDecl();
   if (!clangDecl)
     return AbstractionPattern(fnType);

lib/SILGen/SILGenBridging.cpp

@@ -1315,8 +1315,9 @@ static SILValue emitObjCUnconsumedArgument(SILGenFunction &SGF,
                                            SILLocation loc,
                                            SILValue arg) {
   auto &lowering = SGF.getTypeLowering(arg->getType());
-  // If address-only, make a +1 copy and operate on that.
-  if (lowering.isAddressOnly() && SGF.useLoweredAddresses()) {
+  // If arg is non-trivial and has an address type, make a +1 copy and operate
+  // on that.
+  if (!lowering.isTrivial() && arg->getType().isAddress()) {
     auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
     SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
     return tmp;
@@ -1453,6 +1454,11 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
       auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
       native.forwardInto(SGF, loc, buf);
       native = SGF.emitManagedBufferWithCleanup(buf);
+    } else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
+               native.getType().isAddress()) {
+      // Load the value if the argument has an address type and the native
+      // function expects the argument to be passed directly.
+      native = SGF.emitManagedLoadCopy(loc, native.getValue());
     }
 
     if (nativeInputs[i].isConsumedInCaller()) {

lib/SILGen/SILGenExpr.cpp

@@ -1733,7 +1733,19 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
                                               FunctionConversionExpr *e,
                                               SILType loweredResultTy,
                                 llvm::function_ref<ManagedValue ()> fnEmitter) {
-  SILType loweredDestTy = SGF.getLoweredType(e->getType());
+  SILType loweredDestTy;
+  auto destTy = e->getType();
+  auto clangInfo =
+      destTy->castTo<AnyFunctionType>()->getExtInfo().getClangTypeInfo();
+  if (clangInfo.empty())
+    loweredDestTy = SGF.getLoweredType(destTy);
+  else
+    // This won't be necessary after we stop dropping clang types when
+    // canonicalizing function types.
+    loweredDestTy = SGF.getLoweredType(
+        AbstractionPattern(destTy->getCanonicalType(), clangInfo.getType()),
+        destTy);
+
   ManagedValue result;
 
   // We're converting between C function pointer types. They better be
@@ -1804,20 +1816,20 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
 #endif
     semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr();
   }
-  
+
   if (auto declRef = dyn_cast<DeclRefExpr>(semanticExpr)) {
     setLocFromConcreteDeclRef(declRef->getDeclRef());
   } else if (auto memberRef = dyn_cast<MemberRefExpr>(semanticExpr)) {
     setLocFromConcreteDeclRef(memberRef->getMember());
   } else if (isAnyClosureExpr(semanticExpr)) {
-    (void) emitAnyClosureExpr(SGF, semanticExpr,
-                              [&](AbstractClosureExpr *closure) {
-      // Emit the closure body.
-      SGF.SGM.emitClosure(closure, SGF.getClosureTypeInfo(closure));
+    (void)emitAnyClosureExpr(
+        SGF, semanticExpr, [&](AbstractClosureExpr *closure) {
+          // Emit the closure body.
+          SGF.SGM.emitClosure(closure, SGF.getClosureTypeInfo(closure));
 
-      loc = closure;
-      return ManagedValue();
-    });
+          loc = closure;
+          return ManagedValue();
+        });
   } else {
     llvm_unreachable("c function pointer converted from a non-concrete decl ref");
   }

lib/Serialization/Deserialization.cpp

@@ -8495,9 +8495,6 @@ class SwiftToClangBasicReader :
 
 llvm::Expected<const clang::Type *>
 ModuleFile::getClangType(ClangTypeID TID) {
-  if (!getContext().LangOpts.UseClangFunctionTypes)
-    return nullptr;
-
   if (TID == 0)
     return nullptr;
 

lib/Serialization/Serialization.cpp

@@ -5899,10 +5899,7 @@ class Serializer::TypeSerializer : public TypeVisitor<TypeSerializer> {
     using namespace decls_block;
 
     auto resultType = S.addTypeRef(fnTy->getResult());
-    auto clangType =
-      S.getASTContext().LangOpts.UseClangFunctionTypes
-      ? S.addClangTypeRef(fnTy->getClangTypeInfo().getType())
-      : ClangTypeID(0);
+    auto clangType = S.addClangTypeRef(fnTy->getClangTypeInfo().getType());
 
     auto isolation = encodeIsolation(fnTy->getIsolation());
 

test/Interop/Cxx/class/Inputs/closure.h

@@ -10,6 +10,10 @@ struct NonTrivial {
   int *p;
 };
 
+struct Trivial {
+  int i;
+};
+
 void cfunc(void (^ _Nonnull block)(NonTrivial)) noexcept {
   block(NonTrivial());
 }
@@ -75,4 +79,13 @@ inline void releaseSharedRef(SharedRef *_Nonnull x) {
   }
 }
 
+void cfuncConstRefNonTrivial(void (*_Nonnull)(const NonTrivial &));
+void cfuncConstRefTrivial(void (*_Nonnull)(const Trivial &));
+void blockConstRefNonTrivial(void (^_Nonnull)(const NonTrivial &));
+void blockConstRefTrivial(void (^_Nonnull)(const Trivial &));
+#if __OBJC__
+void cfuncConstRefStrong(void (*_Nonnull)(const ARCStrong &));
+void blockConstRefStrong(void (^_Nonnull)(const ARCStrong &));
+#endif
+
 #endif // __CLOSURE__