Skip to content

Commit 4fdef8b

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 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
1 parent aa16ff5 commit 4fdef8b

19 files changed

+384
-95
lines changed

include/swift/AST/Expr.h

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/AST/FunctionRefInfo.h"
3030
#include "swift/AST/ProtocolConformanceRef.h"
3131
#include "swift/AST/ThrownErrorDestination.h"
32+
#include "swift/AST/Type.h"
3233
#include "swift/AST/TypeAlignments.h"
3334
#include "swift/Basic/Debug.h"
3435
#include "swift/Basic/InlineBitfield.h"
@@ -3368,8 +3369,8 @@ class UnresolvedTypeConversionExpr : public ImplicitConversionExpr {
33683369
class FunctionConversionExpr : public ImplicitConversionExpr {
33693370
public:
33703371
FunctionConversionExpr(Expr *subExpr, Type type)
3371-
: ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {}
3372-
3372+
: ImplicitConversionExpr(ExprKind::FunctionConversion, subExpr, type) {}
3373+
33733374
static bool classof(const Expr *E) {
33743375
return E->getKind() == ExprKind::FunctionConversion;
33753376
}
@@ -4301,19 +4302,19 @@ class ClosureExpr : public AbstractClosureExpr {
43014302
}
43024303

43034304
public:
4304-
ClosureExpr(const DeclAttributes &attributes,
4305-
SourceRange bracketRange, VarDecl *capturedSelfDecl,
4306-
ParameterList *params, SourceLoc asyncLoc, SourceLoc throwsLoc,
4307-
TypeExpr *thrownType, SourceLoc arrowLoc, SourceLoc inLoc,
4308-
TypeExpr *explicitResultType, DeclContext *parent)
4309-
: AbstractClosureExpr(ExprKind::Closure, Type(), /*Implicit=*/false,
4310-
parent),
4311-
Attributes(attributes), BracketRange(bracketRange),
4312-
CapturedSelfDecl(capturedSelfDecl),
4313-
AsyncLoc(asyncLoc), ThrowsLoc(throwsLoc), ArrowLoc(arrowLoc),
4314-
InLoc(inLoc), ThrownType(thrownType),
4315-
ExplicitResultTypeAndBodyState(explicitResultType, BodyState::Parsed),
4316-
Body(nullptr) {
4305+
ClosureExpr(const DeclAttributes &attributes, SourceRange bracketRange,
4306+
VarDecl *capturedSelfDecl, ParameterList *params,
4307+
SourceLoc asyncLoc, SourceLoc throwsLoc, TypeExpr *thrownType,
4308+
SourceLoc arrowLoc, SourceLoc inLoc, TypeExpr *explicitResultType,
4309+
DeclContext *parent)
4310+
: AbstractClosureExpr(ExprKind::Closure, Type(), /*Implicit=*/false,
4311+
parent),
4312+
Attributes(attributes), BracketRange(bracketRange),
4313+
CapturedSelfDecl(capturedSelfDecl), AsyncLoc(asyncLoc),
4314+
ThrowsLoc(throwsLoc), ArrowLoc(arrowLoc), InLoc(inLoc),
4315+
ThrownType(thrownType),
4316+
ExplicitResultTypeAndBodyState(explicitResultType, BodyState::Parsed),
4317+
Body(nullptr) {
43174318
setParameterList(params);
43184319
Bits.ClosureExpr.HasAnonymousClosureVars = false;
43194320
Bits.ClosureExpr.ImplicitSelfCapture = false;

include/swift/SIL/SILBridging.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ struct BridgedSuccessorArray {
974974
};
975975

976976
struct BridgedDeclRef {
977-
uint64_t storage[3];
977+
uint64_t storage[4];
978978

979979
BRIDGED_INLINE BridgedDeclRef(swift::SILDeclRef declRef);
980980
BRIDGED_INLINE swift::SILDeclRef unbridged() const;
@@ -987,7 +987,7 @@ struct BridgedDeclRef {
987987
};
988988

989989
struct BridgedVTableEntry {
990-
uint64_t storage[5];
990+
uint64_t storage[6];
991991

992992
enum class Kind {
993993
Normal,
@@ -1055,7 +1055,7 @@ struct BridgedConstExprFunctionState {
10551055
};
10561056

10571057
struct BridgedWitnessTableEntry {
1058-
uint64_t storage[5];
1058+
uint64_t storage[6];
10591059

10601060
enum class Kind {
10611061
invalid,

include/swift/SIL/SILDeclRef.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ namespace llvm {
3434
class raw_ostream;
3535
}
3636

37+
namespace clang {
38+
class Type;
39+
}
40+
3741
namespace swift {
3842
enum class EffectsKind : uint8_t;
3943
class AbstractFunctionDecl;
@@ -208,6 +212,9 @@ struct SILDeclRef {
208212
const GenericSignatureImpl *, CustomAttr *>
209213
pointer;
210214

215+
// Type of closure thunk.
216+
const clang::Type *thunkType = nullptr;
217+
211218
/// Returns the type of AST node location being stored by the SILDeclRef.
212219
LocKind getLocKind() const {
213220
if (loc.is<ValueDecl *>())
@@ -261,11 +268,10 @@ struct SILDeclRef {
261268
/// for the containing ClassDecl.
262269
/// - If 'loc' is a global VarDecl, this returns its GlobalAccessor
263270
/// SILDeclRef.
264-
explicit SILDeclRef(
265-
Loc loc,
266-
bool isForeign = false,
267-
bool isDistributed = false,
268-
bool isDistributedLocal = false);
271+
explicit SILDeclRef(Loc loc, bool isForeign = false,
272+
bool isDistributed = false,
273+
bool isDistributedLocal = false,
274+
const clang::Type *thunkType = nullptr);
269275

270276
/// See above put produces a prespecialization according to the signature.
271277
explicit SILDeclRef(Loc loc, GenericSignature prespecializationSig);

lib/SIL/IR/SILDeclRef.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,15 @@ SILDeclRef::SILDeclRef(ValueDecl *vd, SILDeclRef::Kind kind, bool isForeign,
135135
isAsyncLetClosure(0), pointer(derivativeId) {}
136136

137137
SILDeclRef::SILDeclRef(SILDeclRef::Loc baseLoc, bool asForeign,
138-
bool asDistributed, bool asDistributedKnownToBeLocal)
138+
bool asDistributed, bool asDistributedKnownToBeLocal,
139+
const clang::Type *thunkType)
139140
: isRuntimeAccessible(false),
140141
backDeploymentKind(SILDeclRef::BackDeploymentKind::None),
141142
defaultArgIndex(0), isAsyncLetClosure(0),
142-
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr) {
143+
pointer((AutoDiffDerivativeFunctionIdentifier *)nullptr),
144+
thunkType(thunkType) {
145+
assert((!thunkType || baseLoc.is<AbstractClosureExpr *>()) &&
146+
"thunk type is needed only for closures");
143147
if (auto *vd = baseLoc.dyn_cast<ValueDecl*>()) {
144148
if (auto *fd = dyn_cast<FuncDecl>(vd)) {
145149
// Map FuncDecls directly to Func SILDeclRefs.

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 12 additions & 4 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"
@@ -4321,10 +4323,12 @@ static CanSILFunctionType getUncachedSILFunctionTypeForConstant(
43214323
// The type of the native-to-foreign thunk for a swift closure.
43224324
if (constant.isForeign && constant.hasClosureExpr() &&
43234325
shouldStoreClangType(TC.getDeclRefRepresentation(constant))) {
4324-
auto clangType = TC.Context.getClangFunctionType(
4325-
origLoweredInterfaceType->getParams(),
4326-
origLoweredInterfaceType->getResult(),
4327-
FunctionTypeRepresentation::CFunctionPointer);
4326+
auto clangType = extInfoBuilder.getClangTypeInfo().getType();
4327+
if (!clangType)
4328+
clangType = TC.Context.getClangFunctionType(
4329+
origLoweredInterfaceType->getParams(),
4330+
origLoweredInterfaceType->getResult(),
4331+
FunctionTypeRepresentation::CFunctionPointer);
43284332
AbstractionPattern pattern =
43294333
AbstractionPattern(origLoweredInterfaceType, clangType);
43304334
return getSILFunctionTypeForAbstractCFunction(
@@ -4834,9 +4838,13 @@ getAbstractionPatternForConstant(ASTContext &ctx, SILDeclRef constant,
48344838
if (!constant.isForeign)
48354839
return AbstractionPattern(fnType);
48364840

4841+
if (constant.thunkType)
4842+
return AbstractionPattern(fnType, constant.thunkType);
4843+
48374844
auto bridgedFn = getBridgedFunction(constant);
48384845
if (!bridgedFn)
48394846
return AbstractionPattern(fnType);
4847+
48404848
const clang::Decl *clangDecl = bridgedFn->getClangDecl();
48414849
if (!clangDecl)
48424850
return AbstractionPattern(fnType);

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
@@ -4258,10 +4258,10 @@ class ArgEmitter {
42584258
loweredSubstArgType,
42594259
param.getSILStorageInterfaceType());
42604260
case SILFunctionLanguage::C:
4261-
return Conversion::getBridging(Conversion::BridgeToObjC,
4262-
arg.getSubstRValueType(),
4263-
origParamType.getType(),
4264-
param.getSILStorageInterfaceType());
4261+
return Conversion::getBridging(
4262+
Conversion::BridgeToObjC, arg.getSubstRValueType(),
4263+
origParamType.getType(), param.getSILStorageInterfaceType(),
4264+
origParamType);
42654265
}
42664266
llvm_unreachable("bad language");
42674267
}();

lib/SILGen/SILGenBridging.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,8 +1315,9 @@ 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()) {
13201321
auto tmp = SGF.emitTemporaryAllocation(loc, arg->getType().getObjectType());
13211322
SGF.B.createCopyAddr(loc, arg, tmp, IsNotTake, IsInitialization);
13221323
return tmp;
@@ -1453,6 +1454,11 @@ emitObjCThunkArguments(SILGenFunction &SGF, SILLocation loc, SILDeclRef thunk,
14531454
auto buf = SGF.emitTemporaryAllocation(loc, native.getType());
14541455
native.forwardInto(SGF, loc, buf);
14551456
native = SGF.emitManagedBufferWithCleanup(buf);
1457+
} else if (!fnConv.isSILIndirect(nativeInputs[i]) &&
1458+
native.getType().isAddress()) {
1459+
// Load the value if the argument has an address type and the native
1460+
// function expects the argument to be passed directly.
1461+
native = SGF.emitManagedLoadCopy(loc, native.getValue());
14561462
}
14571463

14581464
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
}

0 commit comments

Comments
 (0)