Skip to content

Commit 64be397

Browse files
authored
Merge pull request #71933 from jckarter/conditional-copyable-trivial-type-lowering
SIL: More accurate for type lowering whether a type is trivial based on conditional Copyable requirements.
2 parents 99a7a98 + fe7049e commit 64be397

File tree

8 files changed

+215
-15
lines changed

8 files changed

+215
-15
lines changed

include/swift/AST/Module.h

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,7 @@ class ModuleDecl
846846
bool allowMissing = false);
847847

848848
/// Global conformance lookup, checks conditional requirements.
849+
/// Requires a contextualized type.
849850
///
850851
/// \param type The type for which we are computing conformance. Must not
851852
/// contain type parameters.
@@ -859,8 +860,32 @@ class ModuleDecl
859860
/// \returns An invalid conformance if the search failed, otherwise an
860861
/// abstract, concrete or pack conformance, depending on the lookup type.
861862
ProtocolConformanceRef checkConformance(Type type, ProtocolDecl *protocol,
862-
// Note: different default than above
863-
bool allowMissing = true);
863+
// Note: different default from lookupConformance
864+
bool allowMissing = true);
865+
866+
/// Global conformance lookup, checks conditional requirements.
867+
/// Accepts interface types without context. If the conformance cannot be
868+
/// definitively established without the missing context, returns \c nullopt.
869+
///
870+
/// \param type The type for which we are computing conformance. Must not
871+
/// contain type parameters.
872+
///
873+
/// \param protocol The protocol to which we are computing conformance.
874+
///
875+
/// \param allowMissing When \c true, the resulting conformance reference
876+
/// might include "missing" conformances, which are synthesized for some
877+
/// protocols as an error recovery mechanism.
878+
///
879+
/// \returns An invalid conformance if the search definitively failed. An
880+
/// abstract, concrete or pack conformance, depending on the lookup type,
881+
/// if the search succeeded. `std::nullopt` if the type could have
882+
/// conditionally conformed depending on the context of the interface types.
883+
std::optional<ProtocolConformanceRef>
884+
checkConformanceWithoutContext(Type type,
885+
ProtocolDecl *protocol,
886+
// Note: different default from lookupConformance
887+
bool allowMissing = true);
888+
864889

865890
/// Look for the conformance of the given existential type to the given
866891
/// protocol.

include/swift/AST/Requirement.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,12 @@ enum class CheckRequirementsResult : uint8_t {
223223
/// not contain any type parameters.
224224
CheckRequirementsResult checkRequirements(ArrayRef<Requirement> requirements);
225225

226+
/// Check if each substituted requirement is satisfied. If the requirement
227+
/// contains type parameters, and the answer would depend on the context of
228+
/// those type parameters, then `nullopt` is returned.
229+
std::optional<CheckRequirementsResult>
230+
checkRequirementsWithoutContext(ArrayRef<Requirement> requirements);
231+
226232
/// Check if each requirement is satisfied after applying the given
227233
/// substitutions. The substitutions must replace all type parameters that
228234
/// appear in the requirement with concrete types or archetypes.

include/swift/SIL/AbstractionPattern.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,7 @@ class AbstractionPattern {
10141014

10151015
bool requiresClass() const;
10161016
LayoutConstraint getLayoutConstraint() const;
1017+
bool isNoncopyable(CanType substTy) const;
10171018

10181019
/// Return the Swift type which provides structure for this
10191020
/// abstraction pattern.

lib/AST/ConformanceLookup.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -796,8 +796,20 @@ LookupConformanceInModuleRequest::evaluate(
796796
ProtocolConformanceRef
797797
ModuleDecl::checkConformance(Type type, ProtocolDecl *proto,
798798
bool allowMissing) {
799-
assert(!type->hasTypeParameter());
799+
assert(!type->hasTypeParameter()
800+
&& "must take a contextual type. if you really are ok with an "
801+
"indefinite answer (and usually YOU ARE NOT), then consider whether "
802+
"you really, definitely are ok with an indefinite answer, and "
803+
"use `checkConformanceWithoutContext` instead");
804+
805+
// With no type parameter in the type, we should always get a definite answer
806+
// from the underlying test.
807+
return checkConformanceWithoutContext(type, proto, allowMissing).value();
808+
}
800809

810+
std::optional<ProtocolConformanceRef>
811+
ModuleDecl::checkConformanceWithoutContext(Type type, ProtocolDecl *proto,
812+
bool allowMissing) {
801813
auto lookupResult = lookupConformance(type, proto, allowMissing);
802814
if (lookupResult.isInvalid()) {
803815
return ProtocolConformanceRef::forInvalid();
@@ -807,7 +819,11 @@ ModuleDecl::checkConformance(Type type, ProtocolDecl *proto,
807819

808820
// If we have a conditional requirements that we need to check, do so now.
809821
if (!condReqs.empty()) {
810-
switch (checkRequirements(condReqs)) {
822+
auto reqResult = checkRequirementsWithoutContext(condReqs);
823+
if (!reqResult.has_value()) {
824+
return std::nullopt;
825+
}
826+
switch (*reqResult) {
811827
case CheckRequirementsResult::Success:
812828
break;
813829

lib/AST/Requirement.cpp

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,9 @@ int Requirement::compare(const Requirement &other) const {
246246
return compareProtos;
247247
}
248248

249-
CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requirements) {
249+
static std::optional<CheckRequirementsResult>
250+
checkRequirementsImpl(ArrayRef<Requirement> requirements,
251+
bool allowTypeParameters) {
250252
SmallVector<Requirement, 4> worklist(requirements.begin(), requirements.end());
251253

252254
bool hadSubstFailure = false;
@@ -258,12 +260,20 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
258260
#ifndef NDEBUG
259261
{
260262
auto firstType = req.getFirstType();
261-
assert(!firstType->hasTypeParameter());
263+
assert((allowTypeParameters || !firstType->hasTypeParameter())
264+
&& "must take a contextual type. if you really are ok with an "
265+
"indefinite answer (and usually YOU ARE NOT), then consider whether "
266+
"you really, definitely are ok with an indefinite answer, and "
267+
"use `checkRequirementsWithoutContext` instead");
262268
assert(!firstType->hasTypeVariable());
263269

264270
if (req.getKind() != RequirementKind::Layout) {
265271
auto secondType = req.getSecondType();
266-
assert(!secondType->hasTypeParameter());
272+
assert((allowTypeParameters || !secondType->hasTypeParameter())
273+
&& "must take a contextual type. if you really are ok with an "
274+
"indefinite answer (and usually YOU ARE NOT), then consider whether "
275+
"you really, definitely are ok with an indefinite answer, and "
276+
"use `checkRequirementsWithoutContext` instead");
267277
assert(!secondType->hasTypeVariable());
268278
}
269279
}
@@ -276,6 +286,12 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
276286
break;
277287

278288
case CheckRequirementResult::RequirementFailure:
289+
// If a requirement failure was caused by a context-free type parameter,
290+
// then we can't definitely know whether it would have satisfied the
291+
// requirement without context.
292+
if (req.getFirstType()->isTypeParameter()) {
293+
return std::nullopt;
294+
}
279295
return CheckRequirementsResult::RequirementFailure;
280296

281297
case CheckRequirementResult::SubstitutionFailure:
@@ -290,6 +306,19 @@ CheckRequirementsResult swift::checkRequirements(ArrayRef<Requirement> requireme
290306
return CheckRequirementsResult::Success;
291307
}
292308

309+
CheckRequirementsResult
310+
swift::checkRequirements(ArrayRef<Requirement> requirements) {
311+
// This entry point requires that there are no type parameters in any of the
312+
// requirements, so the underlying check should always produce a result.
313+
return checkRequirementsImpl(requirements, /*allow type parameters*/ false)
314+
.value();
315+
}
316+
317+
std::optional<CheckRequirementsResult>
318+
swift::checkRequirementsWithoutContext(ArrayRef<Requirement> requirements) {
319+
return checkRequirementsImpl(requirements, /*allow type parameters*/ true);
320+
}
321+
293322
CheckRequirementsResult swift::checkRequirements(
294323
ModuleDecl *module, ArrayRef<Requirement> requirements,
295324
TypeSubstitutionFn substitutions, SubstOptions options) {
@@ -332,4 +361,4 @@ void InverseRequirement::expandDefaults(
332361
SourceLoc()});
333362
}
334363
}
335-
}
364+
}

lib/SIL/IR/AbstractionPattern.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,83 @@ LayoutConstraint AbstractionPattern::getLayoutConstraint() const {
284284
}
285285
}
286286

287+
bool AbstractionPattern::isNoncopyable(CanType substTy) const {
288+
auto copyable
289+
= substTy->getASTContext().getProtocol(KnownProtocolKind::Copyable);
290+
291+
auto isDefinitelyCopyable = [&](CanType t) -> bool {
292+
auto result = copyable->getParentModule()
293+
->checkConformanceWithoutContext(substTy, copyable,
294+
/*allowMissing=*/false);
295+
return result.has_value() && !result.value().isInvalid();
296+
};
297+
298+
// If the substituted type definitely conforms, that's authoritative.
299+
if (isDefinitelyCopyable(substTy)) {
300+
return false;
301+
}
302+
303+
// If the substituted type is fully concrete, that's it. If there are unbound
304+
// type variables in the type, then we may have to account for the upper
305+
// abstraction bound from the abstraction pattern.
306+
if (!substTy->hasTypeParameter()) {
307+
return true;
308+
}
309+
310+
switch (getKind()) {
311+
case Kind::Opaque: {
312+
// The abstraction pattern doesn't provide any more specific bounds.
313+
return true;
314+
}
315+
case Kind::Type:
316+
case Kind::Discard:
317+
case Kind::ClangType: {
318+
// See whether the abstraction pattern's context gives us an upper bound
319+
// that ensures the type is copyable.
320+
auto type = getType();
321+
if (hasGenericSignature() && getType()->hasTypeParameter()) {
322+
type = GenericEnvironment::mapTypeIntoContext(
323+
getGenericSignature().getGenericEnvironment(), getType())
324+
->getReducedType(getGenericSignature());
325+
}
326+
327+
return !isDefinitelyCopyable(type);
328+
}
329+
case Kind::Tuple: {
330+
// A tuple is noncopyable if any element is.
331+
if (doesTupleVanish()) {
332+
return getVanishingTupleElementPatternType().value()
333+
.isNoncopyable(substTy);
334+
}
335+
auto substTupleTy = cast<TupleType>(substTy);
336+
337+
for (unsigned i = 0, e = getNumTupleElements(); i < e; ++i) {
338+
if (getTupleElementType(i).isNoncopyable(substTupleTy.getElementType(i))){
339+
return true;
340+
}
341+
}
342+
return false;
343+
}
344+
// Functions are, at least for now, always copyable.
345+
case Kind::CurriedObjCMethodType:
346+
case Kind::PartialCurriedObjCMethodType:
347+
case Kind::CFunctionAsMethodType:
348+
case Kind::CurriedCFunctionAsMethodType:
349+
case Kind::PartialCurriedCFunctionAsMethodType:
350+
case Kind::ObjCMethodType:
351+
case Kind::ObjCCompletionHandlerArgumentsType:
352+
case Kind::CXXMethodType:
353+
case Kind::CurriedCXXMethodType:
354+
case Kind::PartialCurriedCXXMethodType:
355+
case Kind::OpaqueFunction:
356+
case Kind::OpaqueDerivativeFunction:
357+
return false;
358+
359+
case Kind::Invalid:
360+
llvm_unreachable("asking invalid abstraction pattern");
361+
}
362+
}
363+
287364
bool AbstractionPattern::matchesTuple(CanType substType) const {
288365
switch (getKind()) {
289366
case Kind::Invalid:

lib/SIL/IR/TypeLowering.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,7 +2353,7 @@ namespace {
23532353

23542354
return handleReference(classType, properties);
23552355
}
2356-
2356+
23572357
// WARNING: when the specification of trivial types changes, also update
23582358
// the isValueTrivial() API used by SILCombine.
23592359
TypeLowering *visitAnyStructType(CanType structType,
@@ -2433,7 +2433,7 @@ namespace {
24332433
properties =
24342434
applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
24352435

2436-
if (D->canBeCopyable() != TypeDecl::CanBeInvertible::Always) {
2436+
if (origType.isNoncopyable(structType)) {
24372437
properties.setNonTrivial();
24382438
properties.setLexical(IsLexical);
24392439
if (properties.isAddressOnly())
@@ -2530,7 +2530,7 @@ namespace {
25302530
properties =
25312531
applyLifetimeAnnotation(D->getLifetimeAnnotation(), properties);
25322532

2533-
if (D->canBeCopyable() != TypeDecl::CanBeInvertible::Always) {
2533+
if (origType.isNoncopyable(enumType)) {
25342534
properties.setNonTrivial();
25352535
properties.setLexical(IsLexical);
25362536
if (properties.isAddressOnly())

test/SILGen/typelowering_inverses.swift

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
// RUN: %target-swift-frontend -emit-silgen -enable-experimental-feature NoncopyableGenerics -disable-availability-checking -module-name main %s | %FileCheck %s
22

3-
4-
53
struct NC: ~Copyable {}
64

75
struct RudeStruct<T: ~Copyable>: Copyable {
@@ -34,13 +32,13 @@ func check(_ t: RudeEnum<Int>) {}
3432
// CHECK: sil hidden [ossa] @$s4main5checkyyAA8RudeEnumOyAA2NCVGF : $@convention(thin) (RudeEnum<NC>) -> () {
3533
func check(_ t: RudeEnum<NC>) {}
3634

37-
// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVySiGF : $@convention(thin) (@guaranteed CondCopyableStruct<Int>) -> () {
35+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVySiGF : $@convention(thin) (CondCopyableStruct<Int>) -> () {
3836
func check(_ t: CondCopyableStruct<Int>) {}
3937

4038
// CHECK: sil hidden [ossa] @$s4main5checkyyAA18CondCopyableStructVyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableStruct<NC>) -> () {
4139
func check(_ t: borrowing CondCopyableStruct<NC>) {}
4240

43-
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOySiGF : $@convention(thin) (@guaranteed CondCopyableEnum<Int>) -> () {
41+
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOySiGF : $@convention(thin) (CondCopyableEnum<Int>) -> () {
4442
func check(_ t: CondCopyableEnum<Int>) {}
4543

4644
// CHECK: sil hidden [ossa] @$s4main5checkyyAA16CondCopyableEnumOyAA2NCVGF : $@convention(thin) (@guaranteed CondCopyableEnum<NC>) -> () {
@@ -51,3 +49,51 @@ func check<T>(_ t: CondCopyableEnum<T>) {}
5149

5250
// CHECK: sil hidden [ossa] @$s4main13check_noClashyyAA16CondCopyableEnumOyxGlF : $@convention(thin) <U where U : ~Copyable> (@in_guaranteed CondCopyableEnum<U>) -> () {
5351
func check_noClash<U: ~Copyable>(_ t: borrowing CondCopyableEnum<U>) {}
52+
53+
struct MyStruct<T: ~Copyable>: ~Copyable {
54+
var x: T
55+
}
56+
57+
extension MyStruct: Copyable where T: Copyable {}
58+
59+
enum MyEnum<T: ~Copyable>: ~Copyable {
60+
case x(T)
61+
case knoll
62+
}
63+
64+
extension MyEnum: Copyable where T: Copyable {}
65+
66+
enum Trivial {
67+
case a, b, c
68+
}
69+
70+
// CHECK-LABEL: sil{{.*}} @{{.*}}13trivialStruct
71+
func trivialStruct() -> Int {
72+
// CHECK: [[ALLOC:%.*]] = alloc_stack $MyStruct<Trivial>
73+
// CHECK-NOT: destroy_addr [[ALLOC]] :
74+
// CHECK: dealloc_stack [[ALLOC]] :
75+
return MemoryLayout.size(ofValue: MyStruct(x: Trivial.a))
76+
}
77+
// CHECK-LABEL: sil{{.*}} @{{.*}}11trivialEnum
78+
func trivialEnum() -> Int {
79+
// CHECK: [[ALLOC:%.*]] = alloc_stack $MyEnum<Trivial>
80+
// CHECK-NOT: destroy_addr [[ALLOC]] :
81+
// CHECK: dealloc_stack [[ALLOC]] :
82+
return MemoryLayout.size(ofValue: MyEnum.x(Trivial.a))
83+
}
84+
85+
struct MyAssortment {
86+
var a: MyStruct<Trivial>
87+
var b: MyEnum<Trivial>
88+
}
89+
90+
// CHECK-LABEL: sil{{.*}} @{{.*}}4frob
91+
func frob(x: MyAssortment) -> Int {
92+
// CHECK: [[ALLOC:%.*]] = alloc_stack $MyAssortment
93+
// CHECK-NOT: destroy_addr [[ALLOC]] :
94+
// CHECK: dealloc_stack [[ALLOC]] :
95+
return MemoryLayout.size(ofValue: x)
96+
}
97+
98+
extension MyEnum: _BitwiseCopyable
99+
where T: Copyable & _BitwiseCopyable {}

0 commit comments

Comments
 (0)