Skip to content

GSB: Start purging the notion of "derived via concrete" requirements #36811

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
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
2 changes: 1 addition & 1 deletion include/swift/AST/GenericSignatureBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ class GenericSignatureBuilder {

class ExplicitRequirement;

bool isRedundantExplicitRequirement(ExplicitRequirement req) const;
bool isRedundantExplicitRequirement(const ExplicitRequirement &req) const;

private:
void computeRedundantRequirements();
Expand Down
79 changes: 51 additions & 28 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,8 @@ struct GenericSignatureBuilder::Implementation {
std::vector<ConflictingConcreteTypeRequirement>
ConflictingConcreteTypeRequirements;

llvm::DenseSet<ExplicitRequirement> ExplicitConformancesImpliedByConcrete;

#ifndef NDEBUG
/// Whether we've already computed redundant requiremnts.
bool computedRedundantRequirements = false;
Expand Down Expand Up @@ -1094,12 +1096,14 @@ const RequirementSource *RequirementSource::getMinimalConformanceSource(
ArchetypeResolutionKind::WellFormed);
assert(parentEquivClass && "Not a well-formed type?");

if (parentEquivClass->concreteType)
derivedViaConcrete = true;
else if (parentEquivClass->superclass &&
builder.lookupConformance(parentEquivClass->superclass,
source->getProtocolDecl()))
derivedViaConcrete = true;
if (requirementSignatureSelfProto) {
if (parentEquivClass->concreteType)
derivedViaConcrete = true;
else if (parentEquivClass->superclass &&
builder.lookupConformance(parentEquivClass->superclass,
source->getProtocolDecl()))
derivedViaConcrete = true;
}

// The parent potential archetype must conform to the protocol in which
// this requirement resides. Add this constraint.
Expand Down Expand Up @@ -6256,6 +6260,20 @@ static bool typeConflictsWithLayoutConstraint(Type t, LayoutConstraint layout) {
return false;
}

static bool isConcreteConformance(const EquivalenceClass &equivClass,
ProtocolDecl *proto,
GenericSignatureBuilder &builder) {
if (equivClass.concreteType &&
builder.lookupConformance(equivClass.concreteType, proto)) {
return true;
} else if (equivClass.superclass &&
builder.lookupConformance(equivClass.superclass, proto)) {
return true;
}

return false;
}

void GenericSignatureBuilder::computeRedundantRequirements() {
assert(!Impl->computedRedundantRequirements &&
"Already computed redundant requirements");
Expand Down Expand Up @@ -6289,6 +6307,14 @@ void GenericSignatureBuilder::computeRedundantRequirements() {

// FIXME: Check for a conflict via the concrete type.
exact.push_back(constraint);

if (!source->isDerivedRequirement()) {
if (isConcreteConformance(equivClass, entry.first, *this)) {
Impl->ExplicitConformancesImpliedByConcrete.insert(
ExplicitRequirement::fromExplicitConstraint(
RequirementKind::Conformance, constraint));
}
}
}

graph.addConstraintsFromEquivClass(RequirementKind::Conformance,
Expand Down Expand Up @@ -7176,6 +7202,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {

for (auto otherReq : found->second) {
auto *otherSource = otherReq.getSource();
if (otherSource->isInferredRequirement())
continue;

auto otherLoc = otherSource->getLoc();
if (otherLoc.isInvalid())
continue;
Expand Down Expand Up @@ -7213,6 +7242,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {

for (auto otherReq : found->second) {
auto *otherSource = otherReq.getSource();
if (otherSource->isInferredRequirement())
continue;

auto otherLoc = otherSource->getLoc();
if (otherLoc.isInvalid())
continue;
Expand Down Expand Up @@ -7252,6 +7284,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {

for (auto otherReq : found->second) {
auto *otherSource = otherReq.getSource();
if (otherSource->isInferredRequirement())
continue;

auto otherLoc = otherSource->getLoc();
if (otherLoc.isInvalid())
continue;
Expand Down Expand Up @@ -8159,7 +8194,7 @@ void GenericSignatureBuilder::checkLayoutConstraints(
}

bool GenericSignatureBuilder::isRedundantExplicitRequirement(
ExplicitRequirement req) const {
const ExplicitRequirement &req) const {
assert(Impl->computedRedundantRequirements &&
"Must ensure computeRedundantRequirements() is called first");
auto &redundantReqs = Impl->RedundantRequirements;
Expand Down Expand Up @@ -8466,23 +8501,6 @@ static void checkGenericSignature(CanGenericSignature canSig,
}
#endif

bool GenericSignatureBuilder::hasExplicitConformancesImpliedByConcrete() const {
for (auto pair : Impl->RedundantRequirements) {
if (pair.first.getKind() != RequirementKind::Conformance)
continue;

for (auto impliedByReq : pair.second) {
if (impliedByReq.getKind() == RequirementKind::Superclass)
return true;

if (impliedByReq.getKind() == RequirementKind::SameType)
return true;
}
}

return false;
}

static Type stripBoundDependentMemberTypes(Type t) {
if (auto *depMemTy = t->getAs<DependentMemberType>()) {
return DependentMemberType::get(
Expand Down Expand Up @@ -8532,7 +8550,8 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
assert(req.getKind() != RequirementKind::SameType &&
"Should not see same-type requirement here");

if (isRedundantExplicitRequirement(req))
if (isRedundantExplicitRequirement(req) &&
Impl->ExplicitConformancesImpliedByConcrete.count(req))
continue;

auto subjectType = req.getSource()->getStoredType();
Expand Down Expand Up @@ -8618,8 +8637,12 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
assert(!Impl->HadAnyError &&
"Rebuilt signature had errors");

assert(!hasExplicitConformancesImpliedByConcrete() &&
"Rebuilt signature still had redundant conformance requirements");
#ifndef NDEBUG
for (const auto &req : Impl->ExplicitConformancesImpliedByConcrete) {
assert(!isRedundantExplicitRequirement(req) &&
"Rebuilt signature still had redundant conformance requirements");
}
#endif
}

// If any of our explicit conformance requirements were implied by
Expand All @@ -8634,7 +8657,7 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
if (!rebuildingWithoutRedundantConformances &&
!buildingRequirementSignature &&
!Impl->HadAnyError &&
hasExplicitConformancesImpliedByConcrete()) {
!Impl->ExplicitConformancesImpliedByConcrete.empty()) {
return std::move(*this).rebuildSignatureWithoutRedundantRequirements(
allowConcreteGenericParams,
buildingRequirementSignature);
Expand Down
8 changes: 8 additions & 0 deletions test/Generics/protocol_superclass_cycle.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: %target-typecheck-verify-swift
// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s

protocol P : C {}
final class C : P {}

// CHECK: Generic signature: <T where T : P>
func foo<T : P>(_: T) {}
4 changes: 3 additions & 1 deletion test/Generics/rdar75656022.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ struct OriginalExampleWithWarning<A, B> where A : P2, B : P2, A.T == B.T {
where C : P1,
D : P1, // expected-warning {{redundant conformance constraint 'D' : 'P1'}}
C.T : P1, // expected-warning {{redundant conformance constraint 'C.T' : 'P1'}}
A == S1<C, C.T.T, S2<C.T>>,
A == S1<C, C.T.T, S2<C.T>>, // expected-note {{conformance constraint 'D' : 'P1' implied here}}
// expected-note@-1 {{conformance constraint 'C.T' : 'P1' implied here}}
C.T == D,
E == D.T { }
}
Expand All @@ -71,6 +72,7 @@ struct WithoutBogusGenericParametersWithWarning<A, B> where A : P2, B : P2, A.T
where C : P1,
C.T : P1, // expected-warning {{redundant conformance constraint 'C.T' : 'P1'}}
A == S1<C, C.T.T, S2<C.T>> {}
// expected-note@-1 {{conformance constraint 'C.T' : 'P1' implied here}}
}

// Same as above but without unnecessary generic parameters
Expand Down