From 4fdef8ba341cb7b5c541fdb1f2d98687cdc3f8ce Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Thu, 20 Mar 2025 15:27:33 +0000 Subject: [PATCH] [SILGen] Fix the type of closure thunks that are passed const reference structs This PR is another attempt at landing #76903. The changes compared to the original PR: * Instead of increasing the size of SILDeclRef, store the necessary type information in the conversion AST nodes. * The PR above introduced a crash during indexing system modules that references foreign types coming from modules imported as implementation only. These entities are implementation details so they do not need to be included during serialization. This PR adds a test and adds logic to exclude such clang types in the serialization process. rdar://131321096&141786724 --- include/swift/AST/Expr.h | 31 +++--- include/swift/SIL/SILBridging.h | 6 +- include/swift/SIL/SILDeclRef.h | 16 ++- lib/SIL/IR/SILDeclRef.cpp | 8 +- lib/SIL/IR/SILFunctionType.cpp | 16 ++- lib/SILGen/Conversion.h | 31 ++++-- lib/SILGen/SILGenApply.cpp | 8 +- lib/SILGen/SILGenBridging.cpp | 10 +- lib/SILGen/SILGenConvert.cpp | 19 ++-- lib/SILGen/SILGenExpr.cpp | 66 ++++++++----- lib/Serialization/Deserialization.cpp | 3 - lib/Serialization/Serialization.cpp | 57 +++++++++-- lib/Serialization/Serialization.h | 2 + test/Interop/Cxx/class/Inputs/closure.h | 13 +++ .../Cxx/class/closure-thunk-macosx.swift | 98 +++++++++++++++++++ ...ntation-only-cxx-module-transitively.swift | 60 ++++++++++++ .../Interop/Cxx/stdlib/use-std-function.swift | 11 +-- .../Inputs/convention_c_function.swift | 3 + .../clang-function-types-convention-c.swift | 21 ++++ 19 files changed, 384 insertions(+), 95 deletions(-) create mode 100644 test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-module-transitively.swift create mode 100644 test/Serialization/Inputs/convention_c_function.swift create mode 100644 test/Serialization/clang-function-types-convention-c.swift diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 7d8e07de69957..8ad6077a267e0 100644 --- a/include/swift/AST/Expr.h +++ b/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" @@ -3368,8 +3369,8 @@ class UnresolvedTypeConversionExpr : public ImplicitConversionExpr { class FunctionConversionExpr : public ImplicitConversionExpr { public: FunctionConversionExpr(Expr *subExpr, Type type) - : ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {} - + : ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {} + static bool classof(const Expr *E) { return E->getKind() == ExprKind::FunctionConversion; } @@ -4301,19 +4302,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) { setParameterList(params); Bits.ClosureExpr.HasAnonymousClosureVars = false; Bits.ClosureExpr.ImplicitSelfCapture = false; diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 5e99a8d88edea..53497b5718a28 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -974,7 +974,7 @@ struct BridgedSuccessorArray { }; struct BridgedDeclRef { - uint64_t storage[3]; + uint64_t storage[4]; BRIDGED_INLINE BridgedDeclRef(swift::SILDeclRef declRef); BRIDGED_INLINE swift::SILDeclRef unbridged() const; @@ -987,7 +987,7 @@ struct BridgedDeclRef { }; struct BridgedVTableEntry { - uint64_t storage[5]; + uint64_t storage[6]; enum class Kind { Normal, @@ -1055,7 +1055,7 @@ struct BridgedConstExprFunctionState { }; struct BridgedWitnessTableEntry { - uint64_t storage[5]; + uint64_t storage[6]; enum class Kind { invalid, diff --git a/include/swift/SIL/SILDeclRef.h b/include/swift/SIL/SILDeclRef.h index d19a66d34659f..fbc756fcfdd7c 100644 --- a/include/swift/SIL/SILDeclRef.h +++ b/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; @@ -208,6 +212,9 @@ struct SILDeclRef { const GenericSignatureImpl *, CustomAttr *> pointer; + // Type of closure thunk. + const clang::Type *thunkType = nullptr; + /// Returns the type of AST node location being stored by the SILDeclRef. LocKind getLocKind() const { if (loc.is()) @@ -261,11 +268,10 @@ 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, + const clang::Type *thunkType = nullptr); /// See above put produces a prespecialization according to the signature. explicit SILDeclRef(Loc loc, GenericSignature prespecializationSig); diff --git a/lib/SIL/IR/SILDeclRef.cpp b/lib/SIL/IR/SILDeclRef.cpp index 07cd8f84365b5..689c7228d6be0 100644 --- a/lib/SIL/IR/SILDeclRef.cpp +++ b/lib/SIL/IR/SILDeclRef.cpp @@ -135,11 +135,15 @@ SILDeclRef::SILDeclRef(ValueDecl *vd, SILDeclRef::Kind kind, bool isForeign, isAsyncLetClosure(0), pointer(derivativeId) {} SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign, - bool asDistributed, bool asDistributedKnownToBeLocal) + bool asDistributed, bool asDistributedKnownToBeLocal, + const clang::Type *thunkType) : isRuntimeAccessible(false), backDeploymentKind(SILDeclRef::BackDeploymentKind::None), defaultArgIndex(0), isAsyncLetClosure(0), - pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr) { + pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr), + thunkType(thunkType) { + assert((!thunkType || baseLoc.is()) && + "thunk type is needed only for closures"); if (auto *vd = baseLoc.dyn_cast()) { if (auto *fd = dyn_cast(vd)) { // Map FuncDecls directly to Func SILDeclRefs. diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index 1e916de1b565c..509317bf730ed 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/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,10 +4323,12 @@ 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); + auto clangType = extInfoBuilder.getClangTypeInfo().getType(); + if (!clangType) + clangType = TC.Context.getClangFunctionType( + origLoweredInterfaceType->getParams(), + origLoweredInterfaceType->getResult(), + FunctionTypeRepresentation::CFunctionPointer); AbstractionPattern pattern = AbstractionPattern(origLoweredInterfaceType, clangType); return getSILFunctionTypeForAbstractCFunction( @@ -4834,9 +4838,13 @@ getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant, if (!constant.isForeign) return AbstractionPattern(fnType); + if (constant.thunkType) + return AbstractionPattern(fnType, constant.thunkType); + auto bridgedFn = getBridgedFunction(constant); if (!bridgedFn) return AbstractionPattern(fnType); + const clang::Decl *clangDecl = bridgedFn->getClangDecl(); if (!clangDecl) return AbstractionPattern(fnType); diff --git a/lib/SILGen/Conversion.h b/lib/SILGen/Conversion.h index 01fe4755a7d27..bd460ecb1b32c 100644 --- a/lib/SILGen/Conversion.h +++ b/lib/SILGen/Conversion.h @@ -17,10 +17,12 @@ #ifndef SWIFT_LOWERING_CONVERSION_H #define SWIFT_LOWERING_CONVERSION_H -#include "swift/Basic/Assertions.h" -#include "swift/Basic/ExternalUnion.h" #include "Initialization.h" #include "SGFContext.h" +#include "swift/Basic/Assertions.h" +#include "swift/Basic/ExternalUnion.h" +#include "swift/SIL/AbstractionPattern.h" +#include namespace swift { namespace Lowering { @@ -115,6 +117,7 @@ class Conversion { struct BridgingStorage { bool IsExplicit; + AbstractionPattern InputOrigType; }; /// The types we store for reabstracting contexts. In general, when @@ -161,11 +164,11 @@ class Conversion { static_assert(decltype(Types)::union_is_trivially_copyable, "define the special members if this changes"); - Conversion(KindTy kind, CanType sourceType, CanType resultType, - SILType loweredResultTy, bool isExplicit) + Conversion(KindTy kind, AbstractionPattern inputOrigType, CanType sourceType, + CanType resultType, SILType loweredResultTy, bool isExplicit) : Kind(kind), SourceType(sourceType), ResultType(resultType), LoweredResultType(loweredResultTy) { - Types.emplaceAggregate(kind, isExplicit); + Types.emplaceAggregate(kind, isExplicit, inputOrigType); } Conversion(AbstractionPattern inputOrigType, CanType inputSubstType, @@ -236,13 +239,19 @@ class Conversion { outputOrigType, outputSubstType, outputLoweredTy); } - static Conversion getBridging(KindTy kind, CanType origType, - CanType resultType, SILType loweredResultTy, - bool isExplicit = false) { + static Conversion + getBridging(KindTy kind, CanType origType, CanType resultType, + SILType loweredResultTy, + std::optional inputOrigType = std::nullopt, + bool isExplicit = false) { assert(isBridgingKind(kind)); assert((kind != Subtype || isAllowedConversion(origType, resultType)) && "disallowed conversion for subtype relationship"); - return Conversion(kind, origType, resultType, loweredResultTy, isExplicit); + if (inputOrigType) + return Conversion(kind, *inputOrigType, origType, resultType, + loweredResultTy, isExplicit); + return Conversion(kind, AbstractionPattern(origType), origType, resultType, + loweredResultTy, isExplicit); } static Conversion getSubtype(CanType origType, CanType substType, @@ -290,6 +299,10 @@ class Conversion { return Types.get(Kind).IsExplicit; } + AbstractionPattern getBridgingOriginalInputType() const { + return Types.get(Kind).InputOrigType; + } + CanType getSourceType() const { return SourceType; } diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index bd8bed5ad56f9..502953e01859b 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -4258,10 +4258,10 @@ class ArgEmitter { loweredSubstArgType, param.getSILStorageInterfaceType()); case SILFunctionLanguage::C: - return Conversion::getBridging(Conversion::BridgeToObjC, - arg.getSubstRValueType(), - origParamType.getType(), - param.getSILStorageInterfaceType()); + return Conversion::getBridging( + Conversion::BridgeToObjC, arg.getSubstRValueType(), + origParamType.getType(), param.getSILStorageInterfaceType(), + origParamType); } llvm_unreachable("bad language"); }(); diff --git a/lib/SILGen/SILGenBridging.cpp b/lib/SILGen/SILGenBridging.cpp index d7cb7c1324009..2579b0e0f013c 100644 --- a/lib/SILGen/SILGenBridging.cpp +++ b/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()) { diff --git a/lib/SILGen/SILGenConvert.cpp b/lib/SILGen/SILGenConvert.cpp index a3f2b8ddadd0a..b84f9bb5e5c00 100644 --- a/lib/SILGen/SILGenConvert.cpp +++ b/lib/SILGen/SILGenConvert.cpp @@ -1348,10 +1348,9 @@ Conversion::adjustForInitialOptionalInjection() const { case BridgeFromObjC: case BridgeResultFromObjC: return OptionalInjectionConversion::forInjection( - getBridging(getKind(), getSourceType().getOptionalObjectType(), - getResultType(), getLoweredResultType(), - isBridgingExplicit()) - ); + getBridging(getKind(), getSourceType().getOptionalObjectType(), + getResultType(), getLoweredResultType(), + getBridgingOriginalInputType(), isBridgingExplicit())); } llvm_unreachable("bad kind"); } @@ -1373,9 +1372,9 @@ Conversion::adjustForInitialOptionalConversions(CanType newSourceType) const { case BridgeToObjC: case BridgeFromObjC: case BridgeResultFromObjC: - return Conversion::getBridging(getKind(), newSourceType, - getResultType(), getLoweredResultType(), - isBridgingExplicit()); + return Conversion::getBridging( + getKind(), newSourceType, getResultType(), getLoweredResultType(), + getBridgingOriginalInputType(), isBridgingExplicit()); } llvm_unreachable("bad kind"); } @@ -1394,9 +1393,9 @@ std::optional Conversion::adjustForInitialForceValue() const { case BridgeToObjC: { auto sourceOptType = getSourceType().wrapInOptionalType(); - return Conversion::getBridging(ForceAndBridgeToObjC, - sourceOptType, getResultType(), - getLoweredResultType(), + return Conversion::getBridging(ForceAndBridgeToObjC, sourceOptType, + getResultType(), getLoweredResultType(), + getBridgingOriginalInputType(), isBridgingExplicit()); } } diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 6be9d7e45f874..4c2b9c072e71f 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -753,10 +753,10 @@ tryEmitAsBridgingConversion(SILGenFunction &SGF, Expr *E, bool isExplicit, auto subExpr = result.SubExpr; CanType resultType = E->getType()->getCanonicalType(); - Conversion conversion = - Conversion::getBridging(kind, subExpr->getType()->getCanonicalType(), - resultType, SGF.getLoweredType(resultType), - isExplicit); + Conversion conversion = Conversion::getBridging( + kind, subExpr->getType()->getCanonicalType(), resultType, + SGF.getLoweredType(resultType), AbstractionPattern(subExpr->getType()), + isExplicit); // Only use this special pattern for AnyErasure conversions when we're // emitting into a peephole. @@ -1733,11 +1733,21 @@ static ManagedValue emitAnyClosureExpr(SILGenFunction &SGF, Expr *e, } } -static ManagedValue convertCFunctionSignature(SILGenFunction &SGF, - FunctionConversionExpr *e, - SILType loweredResultTy, - llvm::function_ref fnEmitter) { - SILType loweredDestTy = SGF.getLoweredType(e->getType()); +static ManagedValue +convertCFunctionSignature(SILGenFunction &SGF, FunctionConversionExpr *e, + SILType loweredResultTy, SGFContext C, + llvm::function_ref fnEmitter) { + SILType loweredDestTy; + auto destTy = e->getType(); + if (const auto init = C.getAsConversion()) { + SILType loweredDestOptTy = init->getConversion().getLoweredResultType(); + if (auto objTy = loweredDestOptTy.getOptionalObjectType()) + loweredDestTy = objTy; + else + loweredDestTy = loweredDestOptTy; + } else + loweredDestTy = SGF.getLoweredType(destTy); + ManagedValue result; // We're converting between C function pointer types. They better be @@ -1772,9 +1782,9 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF, return result; } -static -ManagedValue emitCFunctionPointer(SILGenFunction &SGF, - FunctionConversionExpr *conversionExpr) { +static ManagedValue emitCFunctionPointer(SILGenFunction &SGF, + FunctionConversionExpr *conversionExpr, + SGFContext C) { auto expr = conversionExpr->getSubExpr(); // Look through base-ignored exprs to get to the function ref. @@ -1808,7 +1818,9 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF, #endif semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr(); } - + + const clang::Type *destFnType = nullptr; + if (auto declRef = dyn_cast(semanticExpr)) { setLocFromConcreteDeclRef(declRef->getDeclRef()); } else if (auto memberRef = dyn_cast(semanticExpr)) { @@ -1822,12 +1834,18 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF, loc = closure; return ManagedValue(); }); + if (auto init = C.getAsConversion()) { + auto conv = init->getConversion(); + auto origParamType = conv.getBridgingOriginalInputType(); + if (origParamType.isClangType()) + destFnType = origParamType.getClangType(); + } } else { llvm_unreachable("c function pointer converted from a non-concrete decl ref"); } // Produce a reference to the C-compatible entry point for the function. - SILDeclRef constant(loc, /*foreign*/ true); + SILDeclRef constant(loc, /*foreign*/ true, false, false, destFnType); SILConstantInfo constantInfo = SGF.getConstantInfo(SGF.getTypeExpansionContext(), constant); @@ -1857,13 +1875,10 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF, } return convertCFunctionSignature( - SGF, conversionExpr, - constantInfo.getSILType(), - [&]() -> ManagedValue { - SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant); - return ManagedValue::forObjectRValueWithoutOwnership( - cRef); - }); + SGF, conversionExpr, constantInfo.getSILType(), C, [&]() -> ManagedValue { + SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant); + return ManagedValue::forObjectRValueWithoutOwnership(cRef); + }); } // Change the representation without changing the signature or @@ -2158,7 +2173,7 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e, FunctionTypeRepresentation::CFunctionPointer) { // A "conversion" of a DeclRef a C function pointer is done by referencing // the thunk (or original C function) with the C calling convention. - result = emitCFunctionPointer(SGF, e); + result = emitCFunctionPointer(SGF, e, C); } else { // Ok, we're converting a C function pointer value to another C function // pointer. @@ -2168,10 +2183,9 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e, // Possibly bitcast the C function pointer to account for ABI-compatible // parameter and result type conversions - result = convertCFunctionSignature(SGF, e, result.getType(), - [&]() -> ManagedValue { - return result; - }); + result = + convertCFunctionSignature(SGF, e, result.getType(), C, + [&]() -> ManagedValue { return result; }); } return RValue(SGF, e, result); } diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 6e264e176eea4..4c117d8826db4 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -8508,9 +8508,6 @@ class SwiftToClangBasicReader : llvm::Expected ModuleFile::getClangType(ClangTypeID TID) { - if (!getContext().LangOpts.UseClangFunctionTypes) - return nullptr; - if (TID == 0) return nullptr; diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index b46074388a213..81be98e5b3645 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -17,6 +17,7 @@ #include "swift/AST/ASTMangler.h" #include "swift/AST/ASTVisitor.h" #include "swift/AST/AutoDiff.h" +#include "swift/AST/Decl.h" #include "swift/AST/DiagnosticsCommon.h" #include "swift/AST/DiagnosticsSema.h" #include "swift/AST/Expr.h" @@ -41,6 +42,7 @@ #include "swift/AST/SynthesizedFileUnit.h" #include "swift/AST/TypeCheckRequests.h" #include "swift/AST/TypeVisitor.h" +#include "swift/AST/Types.h" #include "swift/Basic/Assertions.h" #include "swift/Basic/Defer.h" #include "swift/Basic/FileSystem.h" @@ -5553,6 +5555,31 @@ static TypeAliasDecl *findTypeAliasForBuiltin(ASTContext &Ctx, Type T) { return cast(CurModuleResults[0]); } +namespace { +struct ImplementationOnlyWalker : TypeWalker { + bool hadImplementationOnlyDecl = false; + const ModuleDecl *currentModule; + ImplementationOnlyWalker(const ModuleDecl *M) : currentModule(M) {} + Action walkToTypePre(Type ty) override { + if (auto *typeAlias = dyn_cast(ty)) { + if (importedImplementationOnly(typeAlias->getDecl())) + return Action::Stop; + } else if (auto *nominal = ty->getAs()) { + if (importedImplementationOnly(nominal->getDecl())) + return Action::Stop; + } + return Action::Continue; + } + bool importedImplementationOnly(const Decl *D) { + if (currentModule->isImportedImplementationOnly(D->getModuleContext())) { + hadImplementationOnlyDecl = true; + return true; + } + return false; + } +}; +} // namespace + class Serializer::TypeSerializer : public TypeVisitor { Serializer &S; @@ -5906,10 +5933,23 @@ class Serializer::TypeSerializer : public TypeVisitor { using namespace decls_block; auto resultType = S.addTypeRef(fnTy->getResult()); - auto clangType = - S.getASTContext().LangOpts.UseClangFunctionTypes - ? S.addClangTypeRef(fnTy->getClangTypeInfo().getType()) - : ClangTypeID(0); + bool shouldSerializeClangType = true; + if (S.hadImplementationOnlyImport && S.M && + S.M->getResilienceStrategy() != ResilienceStrategy::Resilient) { + // Deserializing clang types from implementation only modules could crash + // as the transitive clang module might not be available to retrieve the + // declarations from. In an optimal world we would make the deseriaization + // more resilient to these problems but the failure is in Clang's + // deserialization code path that is not architected with potentially + // missing declarations in mind. + ImplementationOnlyWalker walker{S.M}; + Type(const_cast(fnTy)).walk(walker); + if (walker.hadImplementationOnlyDecl) + shouldSerializeClangType = false; + } + auto clangType = shouldSerializeClangType + ? S.addClangTypeRef(fnTy->getClangTypeInfo().getType()) + : ClangTypeID(0); auto isolation = encodeIsolation(fnTy->getIsolation()); @@ -6991,8 +7031,13 @@ void Serializer::writeAST(ModuleOrSourceFile DC) { nextFile->getTopLevelDeclsWithAuxiliaryDecls(fileDecls); for (auto D : fileDecls) { - if (isa(D) || isa(D) || - isa(D) || isa(D)) { + if (const auto *ID = dyn_cast(D)) { + if (ID->getAttrs().hasAttribute()) + hadImplementationOnlyImport = true; + continue; + } + if (isa(D) || isa(D) || + isa(D)) { continue; } diff --git a/lib/Serialization/Serialization.h b/lib/Serialization/Serialization.h index df68ffb3c8f51..7e3d3f3102388 100644 --- a/lib/Serialization/Serialization.h +++ b/lib/Serialization/Serialization.h @@ -116,6 +116,8 @@ class Serializer : public SerializerBase { /// an error in the AST. bool hadError = false; + bool hadImplementationOnlyImport = false; + /// Helper for serializing entities in the AST block object graph. /// /// Keeps track of assigning IDs to newly-seen entities, and collecting diff --git a/test/Interop/Cxx/class/Inputs/closure.h b/test/Interop/Cxx/class/Inputs/closure.h index b2c968cf12d89..b842401536f5a 100644 --- a/test/Interop/Cxx/class/Inputs/closure.h +++ b/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__ diff --git a/test/Interop/Cxx/class/closure-thunk-macosx.swift b/test/Interop/Cxx/class/closure-thunk-macosx.swift index 01b459b98aee1..56e34d298a418 100644 --- a/test/Interop/Cxx/class/closure-thunk-macosx.swift +++ b/test/Interop/Cxx/class/closure-thunk-macosx.swift @@ -35,3 +35,101 @@ public func testClosureToFuncPtr() { public func testClosureToBlockReturnNonTrivial() { cfuncReturnNonTrivial({() -> NonTrivial in return NonTrivial() }) } + +// CHECK-LABEL: sil private [thunk] [ossa] @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_To : $@convention(c) (@in_guaranteed NonTrivial) -> () { +// CHECK: bb0(%[[V0:.*]] : $*NonTrivial): +// CHECK: %[[V1:.*]] = alloc_stack $NonTrivial +// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*NonTrivial +// CHECK: %[[V3:.*]] = function_ref @$s4main22testConstRefNonTrivialyyFySo0eF0VcfU_ : $@convention(thin) (@in_guaranteed NonTrivial) -> () +// CHECK: %[[V4:.*]] = apply %[[V3]](%[[V1]]) : $@convention(thin) (@in_guaranteed NonTrivial) -> () +// CHECK: destroy_addr %[[V1]] : $*NonTrivial +// CHECK: dealloc_stack %[[V1]] : $*NonTrivial +// CHECK: return %[[V4]] : $() + +public func testConstRefNonTrivial() { + cfuncConstRefNonTrivial({S in }); +} + +// CHECK-LABEL: sil private [thunk] [ossa] @$s4main23testConstRefNonTrivial2yyFySo0E7TrivialVcfU_To : $@convention(c) (@in_guaranteed NonTrivial) -> () { +// CHECK: bb0(%[[V0:.*]] : $*NonTrivial): +// CHECK: %[[V1:.*]] = alloc_stack $NonTrivial +// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*NonTrivial +// CHECK: %[[V3:.*]] = function_ref @$s4main23testConstRefNonTrivial2yyFySo0E7TrivialVcfU_ : $@convention(thin) (@in_guaranteed NonTrivial) -> () +// CHECK: %[[V4:.*]] = apply %[[V3]](%[[V1]]) : $@convention(thin) (@in_guaranteed NonTrivial) -> () +// CHECK: destroy_addr %[[V1]] : $*NonTrivial +// CHECK: dealloc_stack %[[V1]] : $*NonTrivial +// CHECK: return %[[V4]] : $() +public func testConstRefNonTrivial2() { + cfuncConstRefNonTrivial(({S in })); +} + +// CHECK-LABEL: sil private [thunk] [ossa] @$s4main19testConstRefTrivialyyFySo0E0VcfU_To : $@convention(c) (@in_guaranteed Trivial) -> () { +// CHECK: bb0(%[[V0:.*]] : $*Trivial): +// CHECK: %[[V1:.*]] = load [trivial] %[[V0]] : $*Trivial +// CHECK: %[[V2:.*]] = function_ref @$s4main19testConstRefTrivialyyFySo0E0VcfU_ : $@convention(thin) (Trivial) -> () +// CHECK: %[[V3:.*]] = apply %[[V2]](%[[V1]]) : $@convention(thin) (Trivial) -> () +// CHECK: return %[[V3]] : $() + +public func testConstRefTrivial() { + cfuncConstRefTrivial({S in }); +} + +// CHECK-LABEL: sil private [thunk] [ossa] @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_To : $@convention(c) (@in_guaranteed ARCStrong) -> () { +// CHECK: bb0(%[[V0:.*]] : $*ARCStrong): +// CHECK: %[[V1:.*]] = alloc_stack $ARCStrong +// CHECK: copy_addr %[[V0]] to [init] %[[V1]] : $*ARCStrong +// CHECK: %[[V3:.*]] = load [copy] %[[V1]] : $*ARCStrong +// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $ARCStrong +// CHECK: %[[V5:.*]] = function_ref @$s4main18testConstRefStrongyyFySo9ARCStrongVcfU_ : $@convention(thin) (@guaranteed ARCStrong) -> () +// CHECK: %[[V6:.*]] = apply %[[V5]](%[[V4]]) : $@convention(thin) (@guaranteed ARCStrong) -> () +// CHECK: end_borrow %[[V4]] : $ARCStrong +// CHECK: destroy_value %[[V3]] : $ARCStrong +// CHECK: destroy_addr %[[V1]] : $*ARCStrong +// CHECK: dealloc_stack %[[V1]] : $*ARCStrong +// CHECK: return %[[V6]] : $() + +public func testConstRefStrong() { + cfuncConstRefStrong({S in }); +} + +// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo10NonTrivialVIegn_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), @in_guaranteed NonTrivial) -> () { +// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> (), %[[V1:.*]] : $*NonTrivial): +// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@in_guaranteed NonTrivial) -> () +// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@in_guaranteed NonTrivial) -> () +// CHECK: %[[V4:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> () +// CHECK: apply %[[V4]](%[[V1]]) : $@callee_guaranteed (@in_guaranteed NonTrivial) -> () +// CHECK: end_borrow %[[V4]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> () +// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@in_guaranteed NonTrivial) -> () + +public func testBlockConstRefNonTrivial() { + blockConstRefNonTrivial({S in }); +} + +// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo7TrivialVIegy_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (Trivial) -> (), @in_guaranteed Trivial) -> () { +// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (Trivial) -> (), %[[V1:.*]] : $*Trivial): +// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (Trivial) -> () +// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (Trivial) -> () +// CHECK: %[[V4:.*]] = load [trivial] %[[V1]] : $*Trivial +// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (Trivial) -> () +// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (Trivial) -> () +// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (Trivial) -> () +// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (Trivial) -> () + +public func testBlockConstRefTrivial() { + blockConstRefTrivial({S in }); +} + +// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [ossa] @$sSo9ARCStrongVIegg_ABIeyBn_TR : $@convention(c) (@inout_aliasable @block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), @in_guaranteed ARCStrong) -> () { +// CHECK: bb0(%[[V0:.*]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> (), %[[V1:.*]] : $*ARCStrong): +// CHECK: %[[V2:.*]] = project_block_storage %[[V0]] : $*@block_storage @callee_guaranteed (@guaranteed ARCStrong) -> () +// CHECK: %[[V3:.*]] = load [copy] %[[V2]] : $*@callee_guaranteed (@guaranteed ARCStrong) -> () +// CHECK: %[[V4:.*]] = load_borrow %[[V1]] : $*ARCStrong +// CHECK: %[[V5:.*]] = begin_borrow %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> () +// CHECK: apply %[[V5]](%[[V4]]) : $@callee_guaranteed (@guaranteed ARCStrong) -> () +// CHECK: end_borrow %[[V5]] : $@callee_guaranteed (@guaranteed ARCStrong) -> () +// CHECK: end_borrow %[[V4]] : $ARCStrong +// CHECK: destroy_value %[[V3]] : $@callee_guaranteed (@guaranteed ARCStrong) -> () + +public func testBlockConstRefStrong() { + blockConstRefStrong({S in }); +} diff --git a/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-module-transitively.swift b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-module-transitively.swift new file mode 100644 index 0000000000000..2b216d5284a76 --- /dev/null +++ b/test/Interop/Cxx/implementation-only-imports/import-implementation-only-cxx-module-transitively.swift @@ -0,0 +1,60 @@ +// Crash reproducer from https://github.com/swiftlang/swift/issues/77047#issuecomment-2440103712 +// REQUIRES: OS=macosx + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: mkdir %t/artifacts + +// RUN: %target-swift-frontend -c %t/ChibiStdlib.swift -parse-stdlib -emit-module -emit-module-path %t/sdk/usr/lib/swift/Swift.swiftmodule/%module-target-triple.swiftmodule -module-name Swift -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -o %t/artifacts +// RUN: %target-swift-frontend -c %t/XMLParser.swift -parse-as-library -emit-module -emit-module-path %t/sdk/usr/lib/swift/MyFoundationXML.swiftmodule/%module-target-triple.swiftmodule -module-name MyFoundationXML -module-link-name MyFoundationXML -resource-dir %t/sdk -O -I %t/include -o %t/artifacts -sdk %t/sdk +// RUN: %target-swift-frontend -c %t/Check.swift -o %t/artifacts -index-store-path %t/index-store -index-system-modules -resource-dir %t/sdk -parse-stdlib -sdk %t/sdk + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: mkdir %t/artifacts + +// RUN: %target-swift-frontend -c %t/ChibiStdlib.swift -parse-stdlib -emit-module -emit-module-path %t/sdk/usr/lib/swift/Swift.swiftmodule/%module-target-triple.swiftmodule -module-name Swift -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -o %t/artifacts +// RUN: %target-swift-frontend -c %t/XMLParser.swift -enable-library-evolution -parse-as-library -emit-module -emit-module-path %t/sdk/usr/lib/swift/MyFoundationXML.swiftmodule/%module-target-triple.swiftmodule -module-name MyFoundationXML -module-link-name MyFoundationXML -resource-dir %t/sdk -O -I %t/include -o %t/artifacts -sdk %t/sdk +// RUN: %target-swift-frontend -c %t/Check.swift -o %t/artifacts -index-store-path %t/index-store -index-system-modules -resource-dir %t/sdk -parse-stdlib -sdk %t/sdk + + +//--- Check.swift +import MyFoundationXML + +//--- XMLParser.swift +@_implementationOnly import _MyCFXMLInterface + +func _NSXMLParserExternalEntityWithURL(originalLoaderFunction: _CFXMLInterfaceExternalEntityLoader) {} + +//--- include/module.modulemap +module _MyCFXMLInterface { + header "MyCFXMLInterface.h" +} + +//--- include/MyCFXMLInterface.h +#if !defined(__COREFOUNDATION_CFXMLINTERFACE__) +#define __COREFOUNDATION_CFXMLINTERFACE__ 1 + +typedef struct _xmlParserCtxt *_CFXMLInterfaceParserContext; +typedef void (*_CFXMLInterfaceExternalEntityLoader)(_CFXMLInterfaceParserContext); + +#endif + +//--- ChibiStdlib.swift +precedencegroup AssignmentPrecedence { + assignment: true + associativity: right +} + +public typealias Void = () + +@frozen +public struct OpaquePointer { + @usableFromInline + internal var _rawValue: Builtin.RawPointer + + @usableFromInline @_transparent + internal init(_ v: Builtin.RawPointer) { + self._rawValue = v + } +} diff --git a/test/Interop/Cxx/stdlib/use-std-function.swift b/test/Interop/Cxx/stdlib/use-std-function.swift index ea228ffeb6dc8..4509bc8031d33 100644 --- a/test/Interop/Cxx/stdlib/use-std-function.swift +++ b/test/Interop/Cxx/stdlib/use-std-function.swift @@ -55,12 +55,11 @@ StdFunctionTestSuite.test("FunctionStringToString init from closure and pass as expectEqual(std.string("prefixabcabc"), res) } -// FIXME: assertion for address-only closure params (rdar://124501345) -//StdFunctionTestSuite.test("FunctionStringToStringConstRef init from closure and pass as parameter") { -// let res = invokeFunctionTwiceConstRef(.init({ $0 + std.string("abc") }), -// std.string("prefix")) -// expectEqual(std.string("prefixabcabc"), res) -//} +StdFunctionTestSuite.test("FunctionStringToStringConstRef init from closure and pass as parameter") { + let res = invokeFunctionTwiceConstRef(.init({ $0 + std.string("abc") }), + std.string("prefix")) + expectEqual(std.string("prefixabcabc"), res) +} #endif runAllTests() diff --git a/test/Serialization/Inputs/convention_c_function.swift b/test/Serialization/Inputs/convention_c_function.swift new file mode 100644 index 0000000000000..5ae74013b29ec --- /dev/null +++ b/test/Serialization/Inputs/convention_c_function.swift @@ -0,0 +1,3 @@ +public func foo(fn: @convention(c) () -> ()) -> () { + fn() +} diff --git a/test/Serialization/clang-function-types-convention-c.swift b/test/Serialization/clang-function-types-convention-c.swift new file mode 100644 index 0000000000000..0e1a9b2e9710a --- /dev/null +++ b/test/Serialization/clang-function-types-convention-c.swift @@ -0,0 +1,21 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-module -o %t %S/Inputs/convention_c_function.swift +// RUN: llvm-bcanalyzer %t/convention_c_function.swiftmodule | %FileCheck -check-prefix=CHECK-BCANALYZER %s +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-silgen -I %t %s | %FileCheck %s + +import convention_c_function + +// CHECK-BCANALYZER-LABEL: (INDEX_BLOCK): +// CHECK-BCANALYZER: CLANG_TYPE_OFFSETS + +// Test that the assertion in SILDeclRef doesn't fail. + +// CHECK-LABEL: sil [ossa] @$s4main3baryyF : $@convention(thin) () -> () { +// CHECK: %[[V0:.*]] = function_ref @$s4main3baryyFyycfU_To : $@convention(c) () -> () +// CHECK: %[[V1:.*]] = function_ref @$s21convention_c_function3foo2fnyyyXC_tF : $@convention(thin) (@convention(c) () -> ()) -> () +// CHECK: apply %[[V1]](%[[V0]]) : $@convention(thin) (@convention(c) () -> ()) -> () + +public func bar() { + foo(fn : {}) +} +