Skip to content

GSB: When looking at protocol refinements, only consider other sources on 'Self' #37466

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
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,10 @@ NOTE(redundant_conformance_here,none,
"conformance constraint %0 : %1 implied here",
(Type, ProtocolDecl *))

WARNING(missing_protocol_refinement, none,
"protocol %0 should be declared to refine %1 due to a same-type constraint on 'Self'",
(ProtocolDecl *, ProtocolDecl *))

ERROR(unsupported_recursive_requirements, none,
"requirement involves recursion that is not currently supported", ())

Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/GenericSignatureBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,9 @@ class GenericSignatureBuilder {
void computeRedundantRequirements(
const ProtocolDecl *requirementSignatureSelfProto);

void diagnoseProtocolRefinement(
const ProtocolDecl *requirementSignatureSelfProto);

void diagnoseRedundantRequirements() const;

void diagnoseConflictingConcreteTypeRequirements(
Expand Down
74 changes: 74 additions & 0 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6067,6 +6067,18 @@ void GenericSignatureBuilder::checkIfRequirementCanBeDerived(
}
}

static bool involvesNonSelfSubjectTypes(const RequirementSource *source) {
while (source->kind != RequirementSource::RequirementSignatureSelf) {
if (source->isProtocolRequirement() &&
!source->getStoredType()->is<GenericTypeParamType>())
return true;

source = source->parent;
}

return false;
}

void GenericSignatureBuilder::computeRedundantRequirements(
const ProtocolDecl *requirementSignatureSelfProto) {
assert(!Impl->computedRedundantRequirements &&
Expand Down Expand Up @@ -6126,10 +6138,27 @@ void GenericSignatureBuilder::computeRedundantRequirements(
auto found = equivClass->conformsTo.find(proto);
assert(found != equivClass->conformsTo.end());

// If this is a conformance requirement on 'Self', only consider it
// redundant if it can be derived from other sources involving 'Self'.
//
// What this means in practice is that we will never try to build
// a witness table for an inherited conformance from an associated
// conformance.
//
// This is important since witness tables for inherited conformances
// are realized eagerly before the witness table for the original
// conformance, and allowing more complex requirement sources for
// inherited conformances could introduce cycles at runtime.
bool isProtocolRefinement =
(requirementSignatureSelfProto &&
equivClass->getAnchor(*this, { })->is<GenericTypeParamType>());

checkIfRequirementCanBeDerived(
req, found->second,
requirementSignatureSelfProto,
[&](const Constraint<ProtocolDecl *> &constraint) {
if (isProtocolRefinement)
return involvesNonSelfSubjectTypes(constraint.source);
return false;
});

Expand Down Expand Up @@ -6351,6 +6380,7 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
processDelayedRequirements();

computeRedundantRequirements(requirementSignatureSelfProto);
diagnoseProtocolRefinement(requirementSignatureSelfProto);
diagnoseRedundantRequirements();
diagnoseConflictingConcreteTypeRequirements(requirementSignatureSelfProto);

Expand Down Expand Up @@ -6928,6 +6958,50 @@ static bool isRedundantlyInheritableObjCProtocol(
return true;
}

/// Diagnose any conformance requirements on 'Self' that are not derived from
/// the protocol's inheritance clause.
void GenericSignatureBuilder::diagnoseProtocolRefinement(
const ProtocolDecl *requirementSignatureSelfProto) {
if (requirementSignatureSelfProto == nullptr)
return;

auto selfType = requirementSignatureSelfProto->getSelfInterfaceType();
auto *equivClass = resolveEquivalenceClass(
selfType, ArchetypeResolutionKind::AlreadyKnown);
assert(equivClass != nullptr);

for (auto pair : equivClass->conformsTo) {
auto *proto = pair.first;
bool found = false;
SourceLoc loc;
for (const auto &constraint : pair.second) {
if (!involvesNonSelfSubjectTypes(constraint.source)) {
found = true;
break;
}

auto *parent = constraint.source;
while (parent->kind != RequirementSource::RequirementSignatureSelf) {
loc = parent->getLoc();
if (loc)
break;
}
}

if (!found) {
requirementSignatureSelfProto->diagnose(
diag::missing_protocol_refinement,
const_cast<ProtocolDecl *>(requirementSignatureSelfProto),
proto);

if (loc.isValid()) {
Context.Diags.diagnose(loc, diag::redundant_conformance_here,
selfType, proto);
}
}
}
}

void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
for (const auto &req : Impl->ExplicitRequirements) {
auto *source = req.getSource();
Expand Down
24 changes: 24 additions & 0 deletions test/Generics/redundant_protocol_refinement.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// RUN: %target-typecheck-verify-swift
// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s

protocol Base {}
protocol Middle : Base {}
protocol Derived : Middle, Base {}
// expected-note@-1 {{conformance constraint 'Self' : 'Base' implied here}}
// expected-warning@-2 {{redundant conformance constraint 'Self' : 'Base'}}

protocol P1 {}

protocol P2 {
associatedtype Assoc: P1
}

// no warning here
protocol Good: P2, P1 where Assoc == Self {}
// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self : P2, Self == Self.Assoc>

// missing refinement of 'P1'
protocol Bad: P2 where Assoc == Self {}
// expected-warning@-1 {{protocol 'Bad' should be declared to refine 'P1' due to a same-type constraint on 'Self'}}
// expected-note@-2 {{conformance constraint 'Self' : 'P1' implied here}}
// CHECK-LABEL: Requirement signature: <Self where Self : P2, Self == Self.Assoc>
35 changes: 35 additions & 0 deletions test/Interpreter/sr10941.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift %s -o %t/a.out
// RUN: %target-codesign %t/a.out
// RUN: %target-run %t/a.out
// REQUIRES: executable_test

// https://bugs.swift.org/browse/SR-10941

public protocol SupportedPropertyType { }

public protocol TypedSupportedType: SupportedPropertyType where FactoryType.BuildType == Self {
associatedtype FactoryType: TypedSupportedTypeFactory
}

public protocol TypedSupportedTypeFactory {
associatedtype BuildType: SupportedPropertyType
}

public class Factory<BuildType: TypedSupportedType>: TypedSupportedTypeFactory {
}

public struct ContentMode : TypedSupportedType {
public typealias FactoryType = Factory<ContentMode>
}

func bar<T : SupportedPropertyType>(_: T.Type) {}

func baz<T : TypedSupportedType>(_ t: T.Type) {
bar(t)
}

// This used to recurse infinitely because the base witness table
// for 'ContentMode : SupportedPropertyType' was incorrectly
// minimized away.
baz(ContentMode.self)
10 changes: 10 additions & 0 deletions validation-test/compiler_crashers_2_fixed/rdar57728533.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: not %target-swift-frontend -typecheck %s

protocol Item {
associatedtype Rule
func isValide(valid: Rule) -> Bool
}

protocol PairableItem: Item {
associatedtype AssociatedItem: PairableItem where AssociatedItem.Rule: Rule
}
14 changes: 14 additions & 0 deletions validation-test/compiler_crashers_2_fixed/sr9584.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: not %target-swift-frontend -typecheck %s

struct S<N> {}

protocol P {
associatedtype A: P = Self
static func f(_ x: A) -> A
}

extension S: P where N: P {
static func f<X: P>(_ x: X) -> S<X.A> where A == X, X.A == N {
return S<X.A>()
}
}