Skip to content

Commit f267492

Browse files
author
Gabor Horvath
committed
[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 a side channel using withClosureTypeInfo. * Rely on SGFContext to get the right ClangType * Extend BridgingConversion with an AbstractionPattern to store the original clang type. * 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
1 parent b72ef8e commit f267492

16 files changed

+365
-83
lines changed

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
//
1717
//===----------------------------------------------------------------------===//
1818

19+
#include "swift/AST/Expr.h"
20+
#include "swift/AST/Type.h"
1921
#define DEBUG_TYPE "libsil"
2022

2123
#include "swift/AST/AnyFunctionRef.h"
@@ -4341,10 +4343,12 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
43414343
// The type of the native-to-foreign thunk for a swift closure.
43424344
if (constant.isForeign && constant.hasClosureExpr() &&
43434345
shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
4344-
auto clangType = TC.Context.getClangFunctionType(
4345-
origLoweredInterfaceType->getParams(),
4346-
origLoweredInterfaceType->getResult(),
4347-
FunctionTypeRepresentation::CFunctionPointer);
4346+
auto clangType = extInfoBuilder.getClangTypeInfo().getType();
4347+
if (!clangType)
4348+
clangType = TC.Context.getClangFunctionType(
4349+
origLoweredInterfaceType->getParams(),
4350+
origLoweredInterfaceType->getResult(),
4351+
FunctionTypeRepresentation::CFunctionPointer);
43484352
AbstractionPattern pattern =
43494353
AbstractionPattern(origLoweredInterfaceType, clangType);
43504354
return getSILFunctionTypeForAbstractCFunction(
@@ -4850,15 +4854,21 @@ static AbstractFunctionDecl *getBridgedFunction(SILDeclRef declRef) {
48504854
}
48514855

48524856
static AbstractionPattern
4853-
getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
4854-
CanAnyFunctionType fnType,
4857+
getAbstractionPatternForConstant(TypeConverter &converter, ASTContext &ctx,
4858+
SILDeclRef constant, CanAnyFunctionType fnType,
48554859
unsigned numParameterLists) {
48564860
if (!constant.isForeign)
48574861
return AbstractionPattern(fnType);
48584862

4863+
if (auto *expr = constant.getAbstractClosureExpr()) {
4864+
auto &info = converter.getClosureTypeInfo(expr);
4865+
return info.OrigType;
4866+
}
4867+
48594868
auto bridgedFn = getBridgedFunction(constant);
48604869
if (!bridgedFn)
48614870
return AbstractionPattern(fnType);
4871+
48624872
const clang::Decl *clangDecl = bridgedFn->getClangDecl();
48634873
if (!clangDecl)
48644874
return AbstractionPattern(fnType);
@@ -4901,9 +4911,8 @@ TypeConverter::getLoweredFormalTypes(SILDeclRef constant,
49014911
unsigned numParameterLists = constant.getParameterListCount();
49024912

49034913
// Form an abstraction pattern for bridging purposes.
4904-
AbstractionPattern bridgingFnPattern =
4905-
getAbstractionPatternForConstant(Context, constant, fnType,
4906-
numParameterLists);
4914+
AbstractionPattern bridgingFnPattern = getAbstractionPatternForConstant(
4915+
*this, Context, constant, fnType, numParameterLists);
49074916

49084917
auto extInfo = fnType->getExtInfo();
49094918
SILFunctionTypeRepresentation rep = getDeclRefRepresentation(constant);

lib/SILGen/Conversion.h

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
#ifndef SWIFT_LOWERING_CONVERSION_H
1818
#define SWIFT_LOWERING_CONVERSION_H
1919

20-
#include "swift/Basic/Assertions.h"
21-
#include "swift/Basic/ExternalUnion.h"
2220
#include "Initialization.h"
2321
#include "SGFContext.h"
22+
#include "swift/Basic/Assertions.h"
23+
#include "swift/Basic/ExternalUnion.h"
24+
#include "swift/SIL/AbstractionPattern.h"
25+
#include <optional>
2426

2527
namespace swift {
2628
namespace Lowering {
@@ -115,6 +117,7 @@ class Conversion {
115117

116118
struct BridgingStorage {
117119
bool IsExplicit;
120+
AbstractionPattern InputOrigType;
118121
};
119122

120123
/// The types we store for reabstracting contexts. In general, when
@@ -161,11 +164,11 @@ class Conversion {
161164
static_assert(decltype(Types)::union_is_trivially_copyable,
162165
"define the special members if this changes");
163166

164-
Conversion(KindTy kind, CanType sourceType, CanType resultType,
165-
SILType loweredResultTy, bool isExplicit)
167+
Conversion(KindTy kind, AbstractionPattern inputOrigType, CanType sourceType,
168+
CanType resultType, SILType loweredResultTy, bool isExplicit)
166169
: Kind(kind), SourceType(sourceType), ResultType(resultType),
167170
LoweredResultType(loweredResultTy) {
168-
Types.emplaceAggregate<BridgingStorage>(kind, isExplicit);
171+
Types.emplaceAggregate<BridgingStorage>(kind, isExplicit, inputOrigType);
169172
}
170173

171174
Conversion(AbstractionPattern inputOrigType, CanType inputSubstType,
@@ -236,13 +239,19 @@ class Conversion {
236239
outputOrigType, outputSubstType, outputLoweredTy);
237240
}
238241

239-
static Conversion getBridging(KindTy kind, CanType origType,
240-
CanType resultType, SILType loweredResultTy,
241-
bool isExplicit = false) {
242+
static Conversion
243+
getBridging(KindTy kind, CanType origType, CanType resultType,
244+
SILType loweredResultTy,
245+
std::optional<AbstractionPattern> inputOrigType = std::nullopt,
246+
bool isExplicit = false) {
242247
assert(isBridgingKind(kind));
243248
assert((kind != Subtype || isAllowedConversion(origType, resultType)) &&
244249
"disallowed conversion for subtype relationship");
245-
return Conversion(kind, origType, resultType, loweredResultTy, isExplicit);
250+
if (inputOrigType)
251+
return Conversion(kind, *inputOrigType, origType, resultType,
252+
loweredResultTy, isExplicit);
253+
return Conversion(kind, AbstractionPattern(origType), origType, resultType,
254+
loweredResultTy, isExplicit);
246255
}
247256

248257
static Conversion getSubtype(CanType origType, CanType substType,
@@ -290,6 +299,10 @@ class Conversion {
290299
return Types.get<BridgingStorage>(Kind).IsExplicit;
291300
}
292301

302+
AbstractionPattern getBridgingOriginalInputType() const {
303+
return Types.get<BridgingStorage>(Kind).InputOrigType;
304+
}
305+
293306
CanType getSourceType() const {
294307
return SourceType;
295308
}

lib/SILGen/SILGenApply.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4259,10 +4259,10 @@ class ArgEmitter {
42594259
loweredSubstArgType,
42604260
param.getSILStorageInterfaceType());
42614261
case SILFunctionLanguage::C:
4262-
return Conversion::getBridging(Conversion::BridgeToObjC,
4263-
arg.getSubstRValueType(),
4264-
origParamType.getType(),
4265-
param.getSILStorageInterfaceType());
4262+
return Conversion::getBridging(
4263+
Conversion::BridgeToObjC, arg.getSubstRValueType(),
4264+
origParamType.getType(), param.getSILStorageInterfaceType(),
4265+
origParamType);
42664266
}
42674267
llvm_unreachable("bad language");
42684268
}();

lib/SILGen/SILGenBridging.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,8 +1315,10 @@ static SILValue emitObjCUnconsumedArgument(SILGenFunction &SGF,
13151315
SILLocation loc,
13161316
SILValue arg) {
13171317
auto &lowering = SGF.getTypeLowering(arg->getType());
1318-
// If address-only, make a +1 copy and operate on that.
1319-
if (lowering.isAddressOnly() && SGF.useLoweredAddresses()) {
1318+
// If arg is non-trivial and has an address type, make a +1 copy and operate
1319+
// on that.
1320+
if (!lowering.isTrivial() && arg->getType().isAddress() &&
1321+
SGF.useLoweredAddresses()) {
13201322
auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
13211323
SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
13221324
return tmp;
@@ -1453,6 +1455,11 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
14531455
auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
14541456
native.forwardInto(SGF, loc, buf);
14551457
native = SGF.emitManagedBufferWithCleanup(buf);
1458+
} else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
1459+
native.getType().isAddress() && SGF.useLoweredAddresses()) {
1460+
// Load the value if the argument has an address type and the native
1461+
// function expects the argument to be passed directly.
1462+
native = SGF.emitManagedLoadCopy(loc, native.getValue());
14561463
}
14571464

14581465
if (nativeInputs[i].isConsumedInCaller()) {

lib/SILGen/SILGenConvert.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,10 +1348,9 @@ Conversion::adjustForInitialOptionalInjection() const {
13481348
case BridgeFromObjC:
13491349
case BridgeResultFromObjC:
13501350
return OptionalInjectionConversion::forInjection(
1351-
getBridging(getKind(), getSourceType().getOptionalObjectType(),
1352-
getResultType(), getLoweredResultType(),
1353-
isBridgingExplicit())
1354-
);
1351+
getBridging(getKind(), getSourceType().getOptionalObjectType(),
1352+
getResultType(), getLoweredResultType(),
1353+
getBridgingOriginalInputType(), isBridgingExplicit()));
13551354
}
13561355
llvm_unreachable("bad kind");
13571356
}
@@ -1373,9 +1372,9 @@ Conversion::adjustForInitialOptionalConversions(CanType newSourceType) const {
13731372
case BridgeToObjC:
13741373
case BridgeFromObjC:
13751374
case BridgeResultFromObjC:
1376-
return Conversion::getBridging(getKind(), newSourceType,
1377-
getResultType(), getLoweredResultType(),
1378-
isBridgingExplicit());
1375+
return Conversion::getBridging(
1376+
getKind(), newSourceType, getResultType(), getLoweredResultType(),
1377+
getBridgingOriginalInputType(), isBridgingExplicit());
13791378
}
13801379
llvm_unreachable("bad kind");
13811380
}
@@ -1394,9 +1393,9 @@ std::optional<Conversion> Conversion::adjustForInitialForceValue() const {
13941393

13951394
case BridgeToObjC: {
13961395
auto sourceOptType = getSourceType().wrapInOptionalType();
1397-
return Conversion::getBridging(ForceAndBridgeToObjC,
1398-
sourceOptType, getResultType(),
1399-
getLoweredResultType(),
1396+
return Conversion::getBridging(ForceAndBridgeToObjC, sourceOptType,
1397+
getResultType(), getLoweredResultType(),
1398+
getBridgingOriginalInputType(),
14001399
isBridgingExplicit());
14011400
}
14021401
}

lib/SILGen/SILGenExpr.cpp

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "swift/Basic/Defer.h"
4747
#include "swift/Basic/SourceManager.h"
4848
#include "swift/Basic/type_traits.h"
49+
#include "swift/SIL/AbstractionPattern.h"
4950
#include "swift/SIL/Consumption.h"
5051
#include "swift/SIL/DynamicCasts.h"
5152
#include "swift/SIL/SILArgument.h"
@@ -751,10 +752,10 @@ tryEmitAsBridgingConversion(SILGenFunction &SGF, Expr *E, bool isExplicit,
751752
auto subExpr = result.SubExpr;
752753

753754
CanType resultType = E->getType()->getCanonicalType();
754-
Conversion conversion =
755-
Conversion::getBridging(kind, subExpr->getType()->getCanonicalType(),
756-
resultType, SGF.getLoweredType(resultType),
757-
isExplicit);
755+
Conversion conversion = Conversion::getBridging(
756+
kind, subExpr->getType()->getCanonicalType(), resultType,
757+
SGF.getLoweredType(resultType), AbstractionPattern(subExpr->getType()),
758+
isExplicit);
758759

759760
// Only use this special pattern for AnyErasure conversions when we're
760761
// emitting into a peephole.
@@ -1731,11 +1732,21 @@ static ManagedValue emitAnyClosureExpr(SILGenFunction &SGF, Expr *e,
17311732
}
17321733
}
17331734

1734-
static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
1735-
FunctionConversionExpr *e,
1736-
SILType loweredResultTy,
1737-
llvm::function_ref<ManagedValue ()> fnEmitter) {
1738-
SILType loweredDestTy = SGF.getLoweredType(e->getType());
1735+
static ManagedValue
1736+
convertCFunctionSignature(SILGenFunction &SGF, FunctionConversionExpr *e,
1737+
SILType loweredResultTy, SGFContext C,
1738+
llvm::function_ref<ManagedValue()> fnEmitter) {
1739+
SILType loweredDestTy;
1740+
auto destTy = e->getType();
1741+
if (const auto init = C.getAsConversion()) {
1742+
SILType loweredDestOptTy = init->getConversion().getLoweredResultType();
1743+
if (auto objTy = loweredDestOptTy.getOptionalObjectType())
1744+
loweredDestTy = objTy;
1745+
else
1746+
loweredDestTy = loweredDestOptTy;
1747+
} else
1748+
loweredDestTy = SGF.getLoweredType(destTy);
1749+
17391750
ManagedValue result;
17401751

17411752
// We're converting between C function pointer types. They better be
@@ -1770,9 +1781,9 @@ static ManagedValue convertCFunctionSignature(SILGenFunction &SGF,
17701781
return result;
17711782
}
17721783

1773-
static
1774-
ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
1775-
FunctionConversionExpr *conversionExpr) {
1784+
static ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
1785+
FunctionConversionExpr *conversionExpr,
1786+
SGFContext C) {
17761787
auto expr = conversionExpr->getSubExpr();
17771788

17781789
// Look through base-ignored exprs to get to the function ref.
@@ -1806,20 +1817,33 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18061817
#endif
18071818
semanticExpr = conv->getSubExpr()->getSemanticsProvidingExpr();
18081819
}
1809-
1820+
1821+
const clang::Type *destFnType = nullptr;
1822+
18101823
if (auto declRef = dyn_cast<DeclRefExpr>(semanticExpr)) {
18111824
setLocFromConcreteDeclRef(declRef->getDeclRef());
18121825
} else if (auto memberRef = dyn_cast<MemberRefExpr>(semanticExpr)) {
18131826
setLocFromConcreteDeclRef(memberRef->getMember());
18141827
} else if (isAnyClosureExpr(semanticExpr)) {
1815-
(void) emitAnyClosureExpr(SGF, semanticExpr,
1816-
[&](AbstractClosureExpr *closure) {
1817-
// Emit the closure body.
1818-
SGF.SGM.emitClosure(closure, SGF.getClosureTypeInfo(closure));
1819-
1820-
loc = closure;
1821-
return ManagedValue();
1822-
});
1828+
if (auto init = C.getAsConversion()) {
1829+
auto conv = init->getConversion();
1830+
auto origParamType = conv.getBridgingOriginalInputType();
1831+
if (origParamType.isClangType())
1832+
destFnType = origParamType.getClangType();
1833+
}
1834+
(void)emitAnyClosureExpr(
1835+
SGF, semanticExpr, [&](AbstractClosureExpr *closure) {
1836+
// Emit the closure body.
1837+
auto functionInfo = SGF.getClosureTypeInfo(closure);
1838+
if (destFnType) {
1839+
functionInfo.OrigType =
1840+
AbstractionPattern(functionInfo.OrigType.getType(), destFnType);
1841+
SGF.SGM.Types.withClosureTypeInfo(closure, functionInfo, [] {});
1842+
}
1843+
SGF.SGM.emitClosure(closure, functionInfo);
1844+
loc = closure;
1845+
return ManagedValue::forInContext();
1846+
});
18231847
} else {
18241848
llvm_unreachable("c function pointer converted from a non-concrete decl ref");
18251849
}
@@ -1855,13 +1879,10 @@ ManagedValue emitCFunctionPointer(SILGenFunction &SGF,
18551879
}
18561880

18571881
return convertCFunctionSignature(
1858-
SGF, conversionExpr,
1859-
constantInfo.getSILType(),
1860-
[&]() -> ManagedValue {
1861-
SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant);
1862-
return ManagedValue::forObjectRValueWithoutOwnership(
1863-
cRef);
1864-
});
1882+
SGF, conversionExpr, constantInfo.getSILType(), C, [&]() -> ManagedValue {
1883+
SILValue cRef = SGF.emitGlobalFunctionRef(expr, constant);
1884+
return ManagedValue::forObjectRValueWithoutOwnership(cRef);
1885+
});
18651886
}
18661887

18671888
// Change the representation without changing the signature or
@@ -2118,7 +2139,7 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21182139
FunctionTypeRepresentation::CFunctionPointer) {
21192140
// A "conversion" of a DeclRef a C function pointer is done by referencing
21202141
// the thunk (or original C function) with the C calling convention.
2121-
result = emitCFunctionPointer(SGF, e);
2142+
result = emitCFunctionPointer(SGF, e, C);
21222143
} else {
21232144
// Ok, we're converting a C function pointer value to another C function
21242145
// pointer.
@@ -2128,10 +2149,9 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21282149

21292150
// Possibly bitcast the C function pointer to account for ABI-compatible
21302151
// parameter and result type conversions
2131-
result = convertCFunctionSignature(SGF, e, result.getType(),
2132-
[&]() -> ManagedValue {
2133-
return result;
2134-
});
2152+
result =
2153+
convertCFunctionSignature(SGF, e, result.getType(), C,
2154+
[&]() -> ManagedValue { return result; });
21352155
}
21362156
return RValue(SGF, e, result);
21372157
}

lib/SILGen/SILGenProlog.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1658,7 +1658,8 @@ uint16_t SILGenFunction::emitBasicProlog(
16581658
emitIndirectResultParameters(*this, resultType, origResultType, DC);
16591659

16601660
std::optional<AbstractionPattern> origErrorType;
1661-
if (origClosureType && !origClosureType->isTypeParameterOrOpaqueArchetype()) {
1661+
if (origClosureType && !origClosureType->isTypeParameterOrOpaqueArchetype() &&
1662+
!origClosureType->isClangType()) {
16621663
origErrorType = origClosureType->getFunctionThrownErrorType();
16631664
if (origErrorType && !errorType)
16641665
errorType = origErrorType->getEffectiveThrownErrorType();

lib/Serialization/Deserialization.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8519,9 +8519,6 @@ class SwiftToClangBasicReader :
85198519

85208520
llvm::Expected<const clang::Type *>
85218521
ModuleFile::getClangType(ClangTypeID TID) {
8522-
if (!getContext().LangOpts.UseClangFunctionTypes)
8523-
return nullptr;
8524-
85258522
if (TID == 0)
85268523
return nullptr;
85278524

0 commit comments

Comments
 (0)