Skip to content

Commit e7d7f6f

Browse files
slavapestovslavapestov
committed
RequirementMachine: Add Copyable/Escapable requirements to 'placeholder' generic signatures
If we fail to build a generic signature (or requirement signature of a protocol) because of a request cycle or because Knuth-Bendix completion failed, we would create a placeholder signature with no requirements. However in a move-only world, a completely unconstrained generic parameter might generate spurious diagnostics when used in a copyable way. For this reason, let's outfit these placeholder signatures with a default set of conformance requirements to Copyable and Escapable.
1 parent ff3ea64 commit e7d7f6f

File tree

7 files changed

+87
-27
lines changed

7 files changed

+87
-27
lines changed

‎lib/AST/Decl.cpp

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,9 +1301,10 @@ bool GenericContext::isComputingGenericSignature() const {
13011301

13021302
/// If we hit a cycle while building the generic signature, we can't return
13031303
/// nullptr, since this breaks invariants elsewhere. Instead, build a dummy
1304-
/// signature with no requirements.
1304+
/// signature where everything is Copyable and Escapable, to avoid spurious
1305+
/// downstream diagnostics concerning move-only types.
13051306
static GenericSignature getPlaceholderGenericSignature(
1306-
const DeclContext *DC) {
1307+
ASTContext &ctx, const DeclContext *DC) {
13071308
SmallVector<GenericParamList *, 2> gpLists;
13081309
DC->forEachGenericContext([&](GenericParamList *genericParams) {
13091310
gpLists.push_back(genericParams);
@@ -1316,23 +1317,32 @@ static GenericSignature getPlaceholderGenericSignature(
13161317
for (unsigned i : indices(gpLists))
13171318
gpLists[i]->setDepth(i);
13181319

1319-
SmallVector<GenericTypeParamType *, 2> result;
1320-
for (auto *genericParams : gpLists) {
1321-
for (auto *genericParam : *genericParams) {
1322-
result.push_back(genericParam->getDeclaredInterfaceType()
1323-
->castTo<GenericTypeParamType>());
1320+
SmallVector<GenericTypeParamType *, 2> genericParams;
1321+
SmallVector<Requirement, 2> requirements;
1322+
1323+
for (auto *gpList : gpLists) {
1324+
for (auto *genericParam : *gpList) {
1325+
auto type = genericParam->getDeclaredInterfaceType();
1326+
genericParams.push_back(type->castTo<GenericTypeParamType>());
1327+
1328+
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
1329+
for (auto ip : InvertibleProtocolSet::full()) {
1330+
auto proto = ctx.getProtocol(getKnownProtocolKind(ip));
1331+
requirements.emplace_back(RequirementKind::Conformance, type,
1332+
proto->getDeclaredInterfaceType());
1333+
}
1334+
}
13241335
}
13251336
}
13261337

1327-
return GenericSignature::get(result, {});
1338+
return GenericSignature::get(genericParams, requirements);
13281339
}
13291340

13301341
GenericSignature GenericContext::getGenericSignature() const {
1331-
// Don't use evaluateOrDefault() here, because getting the 'default value'
1332-
// is slightly expensive here so we don't want to do it eagerly.
1333-
return getASTContext().evaluator(
1342+
auto &ctx = getASTContext();
1343+
return ctx.evaluator(
13341344
GenericSignatureRequest{const_cast<GenericContext *>(this)},
1335-
[this]() { return getPlaceholderGenericSignature(this); });
1345+
[&ctx, this]() { return getPlaceholderGenericSignature(ctx, this); });
13361346
}
13371347

13381348
GenericEnvironment *GenericContext::getGenericEnvironment() const {
@@ -6912,10 +6922,46 @@ ProtocolDecl::getProtocolDependencies() const {
69126922
llvm::None);
69136923
}
69146924

6925+
/// If we hit a request cycle, give the protocol a requirement signature where
6926+
/// everything is Copyable and Escapable. Otherwise, we'll get spurious
6927+
/// downstream diagnostics concerning move-only types.
6928+
static RequirementSignature getPlaceholderRequirementSignature(
6929+
const ProtocolDecl *proto) {
6930+
auto &ctx = proto->getASTContext();
6931+
6932+
SmallVector<Requirement, 2> requirements;
6933+
6934+
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
6935+
auto add = [&](Type type) {
6936+
for (auto ip : InvertibleProtocolSet::full()) {
6937+
auto proto = ctx.getProtocol(getKnownProtocolKind(ip));
6938+
requirements.emplace_back(RequirementKind::Conformance, type,
6939+
proto->getDeclaredInterfaceType());
6940+
}
6941+
};
6942+
6943+
add(proto->getSelfInterfaceType());
6944+
6945+
for (auto *assocTypeDecl : proto->getAssociatedTypeMembers())
6946+
add(assocTypeDecl->getDeclaredInterfaceType());
6947+
}
6948+
6949+
// Maintain invariants.
6950+
llvm::array_pod_sort(requirements.begin(), requirements.end(),
6951+
[](const Requirement *lhs, const Requirement *rhs) -> int {
6952+
return lhs->compare(*rhs);
6953+
});
6954+
6955+
return RequirementSignature(ctx.AllocateCopy(requirements),
6956+
ArrayRef<ProtocolTypeAlias>());
6957+
}
6958+
69156959
RequirementSignature ProtocolDecl::getRequirementSignature() const {
6916-
return evaluateOrDefault(getASTContext().evaluator,
6960+
return getASTContext().evaluator(
69176961
RequirementSignatureRequest { const_cast<ProtocolDecl *>(this) },
6918-
RequirementSignature());
6962+
[this]() {
6963+
return getPlaceholderRequirementSignature(this);
6964+
});
69196965
}
69206966

69216967
bool ProtocolDecl::isComputingRequirementSignature() const {

‎lib/AST/Requirement.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,9 @@ void InverseRequirement::expandDefaults(
346346
ASTContext &ctx,
347347
ArrayRef<Type> gps,
348348
SmallVectorImpl<StructuralRequirement> &result) {
349+
if (!SWIFT_ENABLE_EXPERIMENTAL_NONCOPYABLE_GENERICS &&
350+
!ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics))
351+
return;
349352

350353
SmallVector<ProtocolDecl*, NumInvertibleProtocols> defaults;
351354
expandDefaults(ctx, /*inverses=*/{}, defaults);

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,25 @@ AbstractGenericSignatureRequest::evaluate(
746746
}
747747
}
748748

749+
/// If completion fails, build a dummy generic signature where everything is
750+
/// Copyable and Escapable, to avoid spurious downstream diagnostics
751+
/// concerning move-only types.
752+
static GenericSignature getPlaceholderGenericSignature(
753+
ASTContext &ctx, ArrayRef<GenericTypeParamType *> genericParams) {
754+
SmallVector<Requirement, 2> requirements;
755+
if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
756+
for (auto param : genericParams) {
757+
for (auto ip : InvertibleProtocolSet::full()) {
758+
auto proto = ctx.getProtocol(getKnownProtocolKind(ip));
759+
requirements.emplace_back(RequirementKind::Conformance, param,
760+
proto->getDeclaredInterfaceType());
761+
}
762+
}
763+
}
764+
765+
return GenericSignature::get(genericParams, requirements);
766+
}
767+
749768
GenericSignatureWithError
750769
InferredGenericSignatureRequest::evaluate(
751770
Evaluator &evaluator,
@@ -857,7 +876,8 @@ InferredGenericSignatureRequest::evaluate(
857876
// generic parameters, as the outer parameters have already been expanded.
858877
SmallVector<Type, 4> paramTypes;
859878
if (allowInverses) {
860-
paramTypes.append(genericParams.begin() + numOuterParams, genericParams.end());
879+
paramTypes.append(genericParams.begin() + numOuterParams,
880+
genericParams.end());
861881
}
862882

863883
SmallVector<StructuralRequirement, 2> defaults;
@@ -909,8 +929,7 @@ InferredGenericSignatureRequest::evaluate(
909929
diag::requirement_machine_completion_rule,
910930
rule);
911931

912-
auto result = GenericSignature::get(genericParams,
913-
parentSig.getRequirements());
932+
auto result = getPlaceholderGenericSignature(ctx, genericParams);
914933

915934
if (rewriteCtx.getDebugOptions().contains(DebugFlags::Timers)) {
916935
rewriteCtx.endTimer("InferredGenericSignatureRequest");

‎test/Constraints/same_types.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// RUN: %target-typecheck-verify-swift
22
// RUN: not %target-swift-frontend -typecheck %s -debug-generic-signatures 2>&1 | %FileCheck %s
33

4-
// XFAIL: noncopyable_generics
5-
64
protocol Fooable {
75
associatedtype Foo // expected-note{{protocol requires nested type 'Foo'; add nested type 'Foo' for conformance}}
86

‎test/Generics/non_confluent.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
// RUN: %target-typecheck-verify-swift
22

3-
// XFAIL: noncopyable_generics
4-
53
protocol ABA // expected-error {{cannot build rewrite system for protocol; rule length limit exceeded}}
64
// expected-note@-1 {{failed rewrite rule is [ABA:A].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:A] => [ABA:A].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B].[ABA:B]}}
75
where A.B == A.B.A { // expected-error *{{is not a member type}}

‎validation-test/compiler_crashers_2_fixed/0161-issue-49119.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
// RUN: %target-typecheck-verify-swift
22

3-
// XFAIL: noncopyable_generics
4-
53
// https://github.com/apple/swift/issues/49119
64

75
protocol P {

‎validation-test/compiler_crashers_2_fixed/issue-52031.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift
2-
3-
// XFAIL: noncopyable_generics
1+
// RUN: %target-typecheck-verify-swift -requirement-machine-max-rule-length=4
42

53
// https://github.com/apple/swift/issues/52031
64

@@ -14,7 +12,7 @@ protocol P {
1412
extension S: P where N: P {
1513
static func f<X: P>(_ x: X) -> S<X.A> where A == X, X.A == N {
1614
// expected-error@-1 {{cannot build rewrite system for generic signature; rule length limit exceeded}}
17-
// expected-note@-2 {{failed rewrite rule is τ_0_0.[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[concrete: S<S<S<S<S<S<S<S<S<S<S<S<S<S<τ_0_0>>>>>>>>>>>>>>] => τ_0_0.[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A].[P:A] [subst↓]}}
15+
// expected-note@-2 {{τ_0_0.[P:A].[P:A].[P:A].[P:A].[P:A].[concrete: S<S<S<S<S<S<τ_0_0>>>>>>] => τ_0_0.[P:A].[P:A].[P:A].[P:A].[P:A] [subst↓]}}
1816
// expected-error@-3 {{'A' is not a member type of type 'X'}}
1917
// expected-error@-4 {{'A' is not a member type of type 'X'}}
2018
return S<X.A>()

0 commit comments

Comments
 (0)