Skip to content

GSB: Narrow scope of conditional requirement inference #36460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/GenericSignatureBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,9 @@ class GenericSignatureBuilder::FloatingRequirementSource {
/// Whether this is an explicitly-stated requirement.
bool isExplicit() const;

/// Whether this is a derived requirement.
bool isDerived() const;

/// Whether this is a top-level requirement written in source.
/// FIXME: This is a hack because expandConformanceRequirement()
/// is too eager; we should remove this once we fix it properly.
Expand Down
74 changes: 65 additions & 9 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,37 @@ SourceLoc FloatingRequirementSource::getLoc() const {
return SourceLoc();
}

bool FloatingRequirementSource::isDerived() const {
switch (kind) {
case Explicit:
case Inferred:
case NestedTypeNameMatch:
return false;

case AbstractProtocol:
switch (storage.get<const RequirementSource *>()->kind) {
case RequirementSource::RequirementSignatureSelf:
return false;

case RequirementSource::Concrete:
case RequirementSource::Explicit:
case RequirementSource::Inferred:
case RequirementSource::NestedTypeNameMatch:
case RequirementSource::Parent:
case RequirementSource::ProtocolRequirement:
case RequirementSource::InferredProtocolRequirement:
case RequirementSource::Superclass:
case RequirementSource::Layout:
case RequirementSource::EquivalentType:
return true;
}

case Resolved:
return storage.get<const RequirementSource *>()->isDerivedRequirement();
}
llvm_unreachable("unhandled kind");
}

bool FloatingRequirementSource::isExplicit() const {
switch (kind) {
case Explicit:
Expand Down Expand Up @@ -2556,9 +2587,20 @@ GenericSignatureBuilder::resolveConcreteConformance(ResolvedType type,
} else {
concreteSource = concreteSource->viaConcrete(*this, conformance);
equivClass->recordConformanceConstraint(*this, type, proto, concreteSource);
if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
concreteSource->getLoc()))
return nullptr;

// Only infer conditional requirements from explicit sources.
bool hasExplicitSource = llvm::any_of(
equivClass->concreteTypeConstraints,
[](const ConcreteConstraint &constraint) {
return (!constraint.source->isDerivedRequirement() &&
constraint.source->getLoc().isValid());
});

if (hasExplicitSource) {
if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
concreteSource->getLoc()))
return nullptr;
}
}

return concreteSource;
Expand Down Expand Up @@ -2590,9 +2632,20 @@ const RequirementSource *GenericSignatureBuilder::resolveSuperConformance(

superclassSource = superclassSource->viaSuperclass(*this, conformance);
equivClass->recordConformanceConstraint(*this, type, proto, superclassSource);
if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
superclassSource->getLoc()))
return nullptr;

// Only infer conditional requirements from explicit sources.
bool hasExplicitSource = llvm::any_of(
equivClass->superclassConstraints,
[](const ConcreteConstraint &constraint) {
return (!constraint.source->isDerivedRequirement() &&
constraint.source->getLoc().isValid());
});

if (hasExplicitSource) {
if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
superclassSource->getLoc()))
return nullptr;
}

return superclassSource;
}
Expand Down Expand Up @@ -4599,9 +4652,12 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(

// FIXME: diagnose if there's no conformance.
if (conformance) {
if (addConditionalRequirements(conformance, inferForModule,
source.getLoc()))
return ConstraintResult::Conflicting;
// Only infer conditional requirements from explicit sources.
if (!source.isDerived()) {
if (addConditionalRequirements(conformance, inferForModule,
source.getLoc()))
return ConstraintResult::Conflicting;
}
}
}
}
Expand Down
53 changes: 53 additions & 0 deletions test/Generics/conditional_requirement_inference.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// RUN: %target-typecheck-verify-swift
// RUN: not %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s


// Valid example
struct EquatableBox<T : Equatable> {
// CHECK: Generic signature: <T, U where T == Array<U>, U : Equatable>
func withArray<U>(_: U) where T == Array<U> {}
}


// A very elaborate invalid example (see comment in mergeP1AndP2())
struct G<T> {}

protocol P {}
extension G : P where T : P {}

protocol P1 {
associatedtype T
associatedtype U where U == G<T>
associatedtype R : P1
}

protocol P2 {
associatedtype U : P
associatedtype R : P2
}

func takesP<T : P>(_: T.Type) {}
// expected-note@-1 {{where 'T' = 'T.T'}}
// expected-note@-2 {{where 'T' = 'T.R.T'}}
// expected-note@-3 {{where 'T' = 'T.R.R.T'}}
// expected-note@-4 {{where 'T' = 'T.R.R.R.T'}}

// CHECK: Generic signature: <T where T : P1, T : P2>
func mergeP1AndP2<T : P1 & P2>(_: T) {
// P1 implies that T.(R)*.U == G<T.(R)*.T>, and P2 implies that T.(R)*.U : P.
//
// These together would seem to imply that G<T.(R)*.T> : P, therefore
// the conditional conformance G : P should imply that T.(R)*.T : P.
//
// However, this would require us to infer an infinite number of
// conformance requirements in the signature of mergeP1AndP2() of the
// form T.(R)*.T : P.
//
// Since we're unable to represent that, make sure that a) we don't crash,
// b) we reject the conformance T.(R)*.T : P.

takesP(T.T.self) // expected-error {{global function 'takesP' requires that 'T.T' conform to 'P'}}
takesP(T.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.T' conform to 'P'}}
takesP(T.R.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.R.T' conform to 'P'}}
takesP(T.R.R.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.R.R.T' conform to 'P'}}
}
7 changes: 1 addition & 6 deletions test/Generics/validate_stdlib_generic_signatures.swift
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
// Verifies that all of the generic signatures in the standard library are
// minimal and canonical.

// RUN: not %target-typecheck-verify-swift -verify-generic-signatures Swift 2>&1 | %FileCheck %s

// CHECK-NOT: error:
// CHECK-DAG: error: unexpected error produced: generic requirement 'τ_0_0.Index : Strideable' is redundant in <τ_0_0 where τ_0_0 : RandomAccessCollection, τ_0_0.Index : Strideable, τ_0_0.Indices == Range<τ_0_0.Index>, τ_0_0.Index.Stride == Int>
// CHECK-DAG: error: diagnostic produced elsewhere: generic requirement 'τ_0_0.Index : Strideable' is redundant in <τ_0_0 where τ_0_0 : RandomAccessCollection, τ_0_0.Index : Strideable, τ_0_0.Indices == Range<τ_0_0.Index>, τ_0_0.Index.Stride == Int>
// CHECK-NOT: error:
// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift