Skip to content

RequirementMachine: Enable -requirement-machine-inferred-signatures=verify by default #41650

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
68 changes: 65 additions & 3 deletions lib/AST/RequirementMachine/RequirementMachineRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,59 @@ static bool isCanonicalRequest(GenericSignature baseSignature,
return true;
}

/// Hack for GenericSignatureBuilder compatibility. We might end up with a
/// same-type requirement between type parameters where one of them has an
/// implied concrete type requirement. In this case, split it up into two
/// concrete type requirements.
static bool shouldSplitConcreteEquivalenceClass(Requirement req,
GenericSignature sig) {
return (req.getKind() == RequirementKind::SameType &&
req.getSecondType()->isTypeParameter() &&
sig->isConcreteType(req.getSecondType()));
}

static bool shouldSplitConcreteEquivalenceClasses(GenericSignature sig) {
for (auto req : sig.getRequirements()) {
if (shouldSplitConcreteEquivalenceClass(req, sig))
return true;
}

return false;
}

static GenericSignature splitConcreteEquivalenceClasses(
GenericSignature sig, ASTContext &ctx) {
SmallVector<Requirement, 2> reqs;

for (auto req : sig.getRequirements()) {
if (shouldSplitConcreteEquivalenceClass(req, sig)) {
auto canType = sig->getSugaredType(
sig.getCanonicalTypeInContext(
req.getSecondType()));

reqs.emplace_back(RequirementKind::SameType,
req.getFirstType(),
canType);
reqs.emplace_back(RequirementKind::SameType,
req.getSecondType(),
canType);
} else {
reqs.push_back(req);
}
}

SmallVector<GenericTypeParamType *, 2> genericParams;
genericParams.append(sig.getGenericParams().begin(),
sig.getGenericParams().end());

return evaluateOrDefault(
ctx.evaluator,
AbstractGenericSignatureRequestRQM{
/*baseSignature=*/nullptr,
genericParams, reqs},
GenericSignatureWithError()).getPointer();
}

GenericSignatureWithError
AbstractGenericSignatureRequestRQM::evaluate(
Evaluator &evaluator,
Expand Down Expand Up @@ -391,8 +444,13 @@ AbstractGenericSignatureRequestRQM::evaluate(
auto result = GenericSignature::get(genericParams, minimalRequirements);
auto errorFlags = machine->getErrors();

if (!errorFlags)
if (!errorFlags) {
if (shouldSplitConcreteEquivalenceClasses(result))
result = splitConcreteEquivalenceClasses(result, ctx);

// Check invariants.
result.verify();
}

return GenericSignatureWithError(result, errorFlags);
}
Expand Down Expand Up @@ -543,9 +601,13 @@ InferredGenericSignatureRequestRQM::evaluate(

// FIXME: Handle allowConcreteGenericParams

// Check invariants.
if (!errorFlags)
if (!errorFlags) {
if (shouldSplitConcreteEquivalenceClasses(result))
result = splitConcreteEquivalenceClasses(result, ctx);

// Check invariants.
result.verify();
}

return GenericSignatureWithError(result, errorFlags);
}
1 change: 1 addition & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
Args.hasArg(OPT_disable_subst_sil_function_types);

Opts.RequirementMachineProtocolSignatures = RequirementMachineMode::Verify;
Opts.RequirementMachineInferredSignatures = RequirementMachineMode::Verify;
Opts.RequirementMachineAbstractSignatures = RequirementMachineMode::Verify;

if (auto A = Args.getLastArg(OPT_requirement_machine_protocol_signatures_EQ)) {
Expand Down
27 changes: 27 additions & 0 deletions test/Generics/rdar90402519.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %target-typecheck-verify-swift

// This needs a better diagnostic. The real problem is the 'C == G<I>'
// requirement in P2 conflicts with the one in P1.

protocol P {
associatedtype I
}

struct G1<I> : P {}
struct G2<T> : P {
typealias I = G<T>
}

struct G<T> {}

protocol P0 {
associatedtype C : P
}

protocol P1 : P0 where C == G1<I> {
associatedtype I
}

protocol P2 : P1 where C == G2<I> {}
// expected-error@-1 {{cannot build rewrite system for protocol; concrete nesting limit exceeded}}
// expected-note@-2 {{failed rewrite rule is [P2:I].[concrete: G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<G<[P2:I]>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>] => [P2:I]}}
37 changes: 37 additions & 0 deletions test/Generics/split_concrete_equivalence_class.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-inferred-signatures=on 2>&1 | %FileCheck %s

// CHECK-LABEL: split_concrete_equivalence_class.(file).f01@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
func f01<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f02@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
func f02<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f03@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
func f03<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f04@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
func f04<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f05@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Sequence]Element == Character>
func f05<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f06@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Sequence]Element == Character, R.[Collection]SubSequence == Substring>
func f06<C : Collection, R : Collection>(_: C, _: R) where R.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f07@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
func f07<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f08@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
func f08<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element, C.Element == Character {}

// CHECK-LABEL: split_concrete_equivalence_class.(file).f09@
// CHECK-NEXT: Generic signature: <C, R where C : Collection, R : Collection, C.[Collection]SubSequence == Substring, R.[Collection]SubSequence == Substring>
func f09<C : Collection, R : Collection>(_: C, _: R) where C.SubSequence == Substring, R.SubSequence == Substring, C.Element == R.Element, R.Element == Character {}
48 changes: 27 additions & 21 deletions test/Generics/unify_superclass_types_2.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// RUN: %target-typecheck-verify-swift -dump-requirement-machine 2>&1 | %FileCheck %s
// RUN: %target-typecheck-verify-swift -debug-generic-signatures -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s
// RUN: %target-typecheck-verify-swift -dump-requirement-machine -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s --check-prefix=CHECK-RULE

// Note: The GSB fails this test, because it doesn't implement unification of
// superclass type constructor arguments.

// FIXME: The Requirement Machine also fails to minimize the signature of
// unifySuperclassTest(). rdar://90469643

class Generic<T, U, V> {}

protocol P1 {
Expand All @@ -21,30 +25,32 @@ func sameType<T>(_: T.Type, _: T.Type) {}

func takesGenericIntString<U>(_: Generic<Int, String, U>.Type) {}

// CHECK-LABEL: .unifySuperclassTest@
// CHECK-NEXT: Generic signature: <T where T : P1, T : P2>
func unifySuperclassTest<T : P1 & P2>(_: T) {
sameType(T.A1.self, String.self)
sameType(T.A2.self, Int.self)
sameType(T.B1.self, T.B2.self)
takesGenericIntString(T.X.self)
}

// CHECK-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
// CHECK-NEXT: Rewrite system: {
// CHECK: - τ_0_0.[P2:X] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P1:X].[superclass: Generic<τ_0_0.[P2:A2], String, τ_0_0.[P2:B2]>] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
// CHECK: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
// CHECK: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
// CHECK: - τ_0_0.B2 => τ_0_0.[P1:B1]
// CHECK: }
// CHECK: Property map: {
// CHECK-NEXT: [P1] => { conforms_to: [P1] }
// CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, [P1:A1], [P1:B1]>] }
// CHECK-NEXT: [P2] => { conforms_to: [P2] }
// CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
// CHECK-NEXT: τ_0_0 => { conforms_to: [P1 P2] }
// CHECK-NEXT: τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, String, τ_0_0.[P1:B1]>] }
// CHECK-NEXT: τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
// CHECK-NEXT: τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
// CHECK-NEXT: }
// CHECK-RULE-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
// CHECK-RULE-NEXT: Rewrite system: {
// CHECK-RULE: - τ_0_0.[P2:X] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P1:X].[superclass: Generic<τ_0_0.[P2:A2], String, τ_0_0.[P2:B2]>] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
// CHECK-RULE: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
// CHECK-RULE: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
// CHECK-RULE: - τ_0_0.B2 => τ_0_0.[P1:B1]
// CHECK-RULE: }
// CHECK-RULE: Property map: {
// CHECK-RULE-NEXT: [P1] => { conforms_to: [P1] }
// CHECK-RULE-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, [P1:A1], [P1:B1]>] }
// CHECK-RULE-NEXT: [P2] => { conforms_to: [P2] }
// CHECK-RULE-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
// CHECK-RULE-NEXT: τ_0_0 => { conforms_to: [P1 P2] }
// CHECK-RULE-NEXT: τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Generic<Int, String, τ_0_0.[P1:B1]>] }
// CHECK-RULE-NEXT: τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
// CHECK-RULE-NEXT: τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
// CHECK-RULE-NEXT: }
52 changes: 29 additions & 23 deletions test/Generics/unify_superclass_types_3.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// RUN: %target-typecheck-verify-swift -dump-requirement-machine 2>&1 | %FileCheck %s
// RUN: %target-typecheck-verify-swift -debug-generic-signatures -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s
// RUN: %target-typecheck-verify-swift -dump-requirement-machine -requirement-machine-inferred-signatures=off 2>&1 | %FileCheck %s --check-prefix=CHECK-RULE

// Note: The GSB fails this test, because it doesn't implement unification of
// superclass type constructor arguments.

// FIXME: The Requirement Machine also fails to minimize the signature of
// unifySuperclassTest(). rdar://90469643

class Generic<T, U, V> {}

class Derived<TT, UU> : Generic<Int, TT, UU> {}
Expand All @@ -23,32 +27,34 @@ func sameType<T>(_: T.Type, _: T.Type) {}

func takesDerivedString<U>(_: Derived<String, U>.Type) {}

// CHECK-LABEL: .unifySuperclassTest@
// CHECK-NEXT: Generic signature: <T where T : P1, T : P2>
func unifySuperclassTest<T : P1 & P2>(_: T) {
sameType(T.A1.self, String.self)
sameType(T.A2.self, Int.self)
sameType(T.B1.self, T.B2.self)
takesDerivedString(T.X.self)
}

// CHECK-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
// CHECK-NEXT: Rewrite system: {
// CHECK: - [P1:X].[layout: _NativeClass] => [P1:X]
// CHECK: - [P2:X].[layout: _NativeClass] => [P2:X]
// CHECK: - τ_0_0.[P2:X] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P1:X].[superclass: Generic<Int, τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
// CHECK: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
// CHECK: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
// CHECK: - τ_0_0.B2 => τ_0_0.[P1:B1]
// CHECK: }
// CHECK: Property map: {
// CHECK-NEXT: [P1] => { conforms_to: [P1] }
// CHECK-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Derived<[P1:A1], [P1:B1]>] }
// CHECK-NEXT: [P2] => { conforms_to: [P2] }
// CHECK-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
// CHECK-NEXT: τ_0_0 => { conforms_to: [P1 P2] }
// CHECK-NEXT: τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] }
// CHECK-NEXT: τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
// CHECK-NEXT: τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
// CHECK-NEXT: }
// CHECK-RULE-LABEL: Requirement machine for <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2>
// CHECK-RULE-NEXT: Rewrite system: {
// CHECK-RULE: - [P1:X].[layout: _NativeClass] => [P1:X]
// CHECK-RULE: - [P2:X].[layout: _NativeClass] => [P2:X]
// CHECK-RULE: - τ_0_0.[P2:X] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P1:X].[superclass: Generic<Int, τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P1:X].[superclass: Generic<Int, String, τ_0_0.[P1:B1]>] => τ_0_0.[P1:X]
// CHECK-RULE: - τ_0_0.[P2:A2].[concrete: Int] => τ_0_0.[P2:A2]
// CHECK-RULE: - τ_0_0.[P2:B2] => τ_0_0.[P1:B1]
// CHECK-RULE: - τ_0_0.[P1:A1].[concrete: String] => τ_0_0.[P1:A1]
// CHECK-RULE: - τ_0_0.B2 => τ_0_0.[P1:B1]
// CHECK-RULE: }
// CHECK-RULE: Property map: {
// CHECK-RULE-NEXT: [P1] => { conforms_to: [P1] }
// CHECK-RULE-NEXT: [P1:X] => { layout: _NativeClass superclass: [superclass: Derived<[P1:A1], [P1:B1]>] }
// CHECK-RULE-NEXT: [P2] => { conforms_to: [P2] }
// CHECK-RULE-NEXT: [P2:X] => { layout: _NativeClass superclass: [superclass: Generic<[P2:A2], String, [P2:B2]>] }
// CHECK-RULE-NEXT: τ_0_0 => { conforms_to: [P1 P2] }
// CHECK-RULE-NEXT: τ_0_0.[P1:X] => { layout: _NativeClass superclass: [superclass: Derived<τ_0_0.[P1:A1], τ_0_0.[P1:B1]>] }
// CHECK-RULE-NEXT: τ_0_0.[P2:A2] => { concrete_type: [concrete: Int] }
// CHECK-RULE-NEXT: τ_0_0.[P1:A1] => { concrete_type: [concrete: String] }
// CHECK-RULE-NEXT: }
28 changes: 0 additions & 28 deletions test/attr/accessibility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,31 +207,3 @@ extension GenericStruct where Param: InternalProto {
public func foo() {} // expected-error {{cannot declare a public instance method in an extension with internal requirements}} {{3-9=internal}}
}


public class OuterClass {
class InnerClass {}
}

public protocol PublicProto2 {
associatedtype T
associatedtype U
}

// FIXME: With the current design, the below should not diagnose.
//
// However, it does, because we look at the bound decl in the
// TypeRepr first, and it happens to already be set.
//
// FIXME: Once we no longer do that, come up with another strategy
// to make the above diagnose.

extension PublicProto2 where Self.T : OuterClass, Self.U == Self.T.InnerClass {
public func cannotBePublic() {}
// expected-error@-1 {{cannot declare a public instance method in an extension with internal requirements}}
}

public extension OuterClass {
open convenience init(x: ()) { self.init() }
// expected-warning@-1 {{'open' modifier conflicts with extension's default access of 'public'}}
// expected-error@-2 {{only classes and overridable class members can be declared 'open'; use 'public'}}
}
32 changes: 32 additions & 0 deletions test/attr/accessibility_where_clause.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// RUN: %target-typecheck-verify-swift -requirement-machine-inferred-signatures=off

public class OuterClass {
class InnerClass {}
}

public protocol PublicProto2 {
associatedtype T
associatedtype U
}

// FIXME: With the current design, the below should not diagnose.
//
// However, it does, because we look at the bound decl in the
// TypeRepr first, and it happens to already be set.
//
// FIXME: Once we no longer do that, come up with another strategy
// to make the above diagnose.

// FIXME: Get this working with the Requirement Machine, or decide that it should
// be unsupported: rdar://90469477

extension PublicProto2 where Self.T : OuterClass, Self.U == Self.T.InnerClass {
public func cannotBePublic() {}
// expected-error@-1 {{cannot declare a public instance method in an extension with internal requirements}}
}

public extension OuterClass {
open convenience init(x: ()) { self.init() }
// expected-warning@-1 {{'open' modifier conflicts with extension's default access of 'public'}}
// expected-error@-2 {{only classes and overridable class members can be declared 'open'; use 'public'}}
}