Skip to content

Commit 4657fa9

Browse files
committed
[NCGenerics] fix type lowering
TypeDecl::canBeNoncopyable is never going to be fully accurate. In this instance, we were incorrectly treating some types like `S<T: ~Copyable>` as noncopyable, and thus non-trivial, despite them in some cases being Copyable.
1 parent 5e0bcab commit 4657fa9

File tree

6 files changed

+86
-17
lines changed

6 files changed

+86
-17
lines changed

include/swift/AST/Types.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,11 +647,19 @@ class alignas(1 << TypeAlignInBits) TypeBase
647647

648648
/// Returns true if this type lacks conformance to Copyable in the context,
649649
/// if provided.
650-
bool isNoncopyable(const DeclContext *dc = nullptr);
650+
bool isNoncopyable(GenericEnvironment *env = nullptr);
651+
bool isNoncopyable(const DeclContext *dc) {
652+
assert(dc);
653+
return isNoncopyable(dc->getGenericEnvironmentOfContext());
654+
};
651655

652656
/// Returns true if this type conforms to Escapable in the context,
653657
/// if provided.
654-
bool isEscapable(const DeclContext *dc = nullptr);
658+
bool isEscapable(GenericEnvironment *env = nullptr);
659+
bool isEscapable(const DeclContext *dc) {
660+
assert(dc);
661+
return isEscapable(dc->getGenericEnvironmentOfContext());
662+
};
655663

656664
/// Does the type have outer parenthesis?
657665
bool hasParenSugar() const { return getKind() == TypeKind::Paren; }

lib/AST/ASTPrinter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1737,7 +1737,8 @@ void PrintAST::printGenericSignature(
17371737
genericParams.slice(paramIdx, lastParamIdx - paramIdx);
17381738

17391739
SmallVector<InverseRequirement, 2> inversesAtDepth;
1740-
reconstituteInverses(genericSig, genericParamsAtDepth, inversesAtDepth);
1740+
if (flags & CollapseDefaultReqs)
1741+
reconstituteInverses(genericSig, genericParamsAtDepth, inversesAtDepth);
17411742

17421743
SmallVector<Requirement, 2> requirementsAtDepth;
17431744
getRequirementsAtDepth(genericSig, depth, requirementsAtDepth);

lib/AST/Decl.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4910,12 +4910,13 @@ InverseMarking TypeDecl::getMarking(InvertibleProtocolKind ip) const {
49104910
}
49114911

49124912
bool TypeDecl::canBeNoncopyable() const {
4913-
return getMarking(InvertibleProtocolKind::Copyable).getInverse().isPresent();
4913+
auto copyable = getMarking(InvertibleProtocolKind::Copyable);
4914+
return bool(copyable.getInverse()) && !copyable.getPositive();
49144915
}
49154916

49164917
bool TypeDecl::isEscapable() const {
49174918
auto escapable = getMarking(InvertibleProtocolKind::Escapable);
4918-
return !escapable.getInverse().isPresent();
4919+
return !escapable.getInverse() || bool(escapable.getPositive());
49194920
}
49204921

49214922
Type TypeDecl::getDeclaredInterfaceType() const {

lib/AST/Type.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -177,24 +177,22 @@ static bool alwaysNoncopyable(Type ty) {
177177
return false; // otherwise, the conservative assumption is it's copyable.
178178
}
179179

180-
static CanType preprocessType(const DeclContext *dc, Type orig) {
180+
static CanType preprocessType(GenericEnvironment *env, Type orig) {
181181
Type type = orig;
182182

183183
// Turn any type parameters into archetypes.
184-
if (dc) {
184+
if (env)
185185
if (!type->hasArchetype() || type->hasOpenedExistential())
186-
if (auto env = dc->getGenericEnvironmentOfContext())
187-
type = GenericEnvironment::mapTypeIntoContext(env, type);
188-
}
186+
type = GenericEnvironment::mapTypeIntoContext(env, type);
189187

190188
// Strip @lvalue and canonicalize.
191189
auto canType = type->getRValueType()->getCanonicalType();
192190
return canType;
193191
}
194192

195193
/// \returns true iff this type lacks conformance to Copyable.
196-
bool TypeBase::isNoncopyable(const DeclContext *dc) {
197-
auto canType = preprocessType(dc, this);
194+
bool TypeBase::isNoncopyable(GenericEnvironment *env) {
195+
auto canType = preprocessType(env, this);
198196
auto &ctx = canType->getASTContext();
199197

200198
// for legacy-mode queries that are not dependent on conformances to Copyable
@@ -205,8 +203,8 @@ bool TypeBase::isNoncopyable(const DeclContext *dc) {
205203
return evaluateOrDefault(ctx.evaluator, request, /*default=*/true);
206204
}
207205

208-
bool TypeBase::isEscapable(const DeclContext *dc) {
209-
auto canType = preprocessType(dc, this);
206+
bool TypeBase::isEscapable(GenericEnvironment *env) {
207+
auto canType = preprocessType(env, this);
210208
auto &ctx = canType->getASTContext();
211209

212210
// for legacy-mode queries that are not dependent on conformances to Escapable

lib/SIL/IR/TypeLowering.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,15 +2406,19 @@ namespace {
24062406
properties =
24072407
applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
24082408

2409-
if (D->canBeNoncopyable()) {
2409+
GenericEnvironment *env = nullptr;
2410+
if (auto sig = origType.getGenericSignatureOrNull())
2411+
env = sig.getGenericEnvironment();
2412+
2413+
if (structType->isNoncopyable(env)) {
24102414
properties.setNonTrivial();
24112415
properties.setLexical(IsLexical);
24122416
if (properties.isAddressOnly())
24132417
return handleMoveOnlyAddressOnly(structType, properties);
24142418
return new (TC) MoveOnlyLoadableStructTypeLowering(
24152419
structType, properties, Expansion);
24162420
}
2417-
if (!D->isEscapable()) {
2421+
if (!structType->isEscapable(env)) {
24182422
properties.setNonTrivial();
24192423
}
24202424
return handleAggregateByProperties<LoadableStructTypeLowering>(structType,
@@ -2493,7 +2497,11 @@ namespace {
24932497
properties =
24942498
applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
24952499

2496-
if (D->canBeNoncopyable()) {
2500+
GenericEnvironment *env = nullptr;
2501+
if (auto sig = origType.getGenericSignatureOrNull())
2502+
env = sig.getGenericEnvironment();
2503+
2504+
if (enumType->isNoncopyable(env)) {
24972505
properties.setNonTrivial();
24982506
properties.setLexical(IsLexical);
24992507
if (properties.isAddressOnly())
@@ -2502,6 +2510,8 @@ namespace {
25022510
MoveOnlyLoadableEnumTypeLowering(enumType, properties, Expansion);
25032511
}
25042512

2513+
assert(enumType->isEscapable(env) && "missing typelowering case here!");
2514+
25052515
return handleAggregateByProperties<LoadableEnumTypeLowering>(enumType,
25062516
properties);
25072517
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// RUN: %target-swift-frontend -emit-silgen -enable-experimental-feature NoncopyableGenerics -disable-availability-checking -module-name main %s | %FileCheck %s
2+
3+
struct NC: ~Copyable {}
4+
5+
struct RudeStruct<T: ~Copyable>: Copyable {
6+
let thing: Int
7+
}
8+
9+
enum RudeEnum<T: ~Copyable>: Copyable {
10+
case holder(Int)
11+
case whatever
12+
}
13+
14+
struct CondCopyableStruct<T: ~Copyable> {}
15+
16+
enum CondCopyableEnum<T: ~Copyable> {
17+
case some(T)
18+
case none
19+
}
20+
21+
// MARK: ensure certain types are treated as trivial (no ownership in func signature).
22+
23+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA10RudeStructVySiGF : $@convention(thin) (RudeStruct<Int>) -> () {
24+
func check(_ t: RudeStruct<Int>) {}
25+
26+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA10RudeStructVyAA2NCVGF : $@convention(thin) (RudeStruct<NC>) -> () {
27+
func check(_ t: RudeStruct<NC>) {}
28+
29+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA8RudeEnumOySiGF : $@convention(thin) (RudeEnum<Int>) -> () {
30+
func check(_ t: RudeEnum<Int>) {}
31+
32+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA8RudeEnumOyAA2NCVGF : $@convention(thin) (RudeEnum<NC>) -> () {
33+
func check(_ t: RudeEnum<NC>) {}
34+
35+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVySiGF : $@convention(thin) (CondCopyableStruct<Int>) -> () {
36+
func check(_ t: CondCopyableStruct<Int>) {}
37+
38+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableStruct<NC>) -> () {
39+
func check(_ t: borrowing CondCopyableStruct<NC>) {}
40+
41+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOySiGF : $@convention(thin) (CondCopyableEnum<Int>) -> () {
42+
func check(_ t: CondCopyableEnum<Int>) {}
43+
44+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableEnum<NC>) -> () {
45+
func check(_ t: borrowing CondCopyableEnum<NC>) {}
46+
47+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOyxGs0D0Rzs9EscapableRzlF : $@convention(thin) <T where T : Copyable, T : Escapable> (@in_guaranteed CondCopyableEnum<T>) -> () {
48+
func check<T>(_ t: CondCopyableEnum<T>) {}
49+
50+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOyxGs9EscapableRzlF : $@convention(thin) <U where U : Escapable> (@in_guaranteed CondCopyableEnum<U>) -> () {
51+
func check<U: ~Copyable>(_ t: borrowing CondCopyableEnum<U>) {}

0 commit comments

Comments
 (0)