Skip to content

Commit a7c698c

Browse files
slavapestovslavapestov
authored
Merge pull request #41324 from slavapestov/rqm-trivial-validation-test-fixes
RequirementMachine: Fix some trivial problems running validation tests with -requirement-machine-inferred-signatures=verify
2 parents 6ea8763 + 1244b17 commit a7c698c

File tree

8 files changed

+153
-67
lines changed

8 files changed

+153
-67
lines changed

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8391,9 +8391,16 @@ AbstractGenericSignatureRequest::evaluate(
83918391
return rqmResult;
83928392

83938393
if (!rqmResult.getPointer()->isEqual(gsbResult.getPointer())) {
8394+
PrintOptions opts;
8395+
opts.ProtocolQualifiedDependentMemberTypes = true;
8396+
83948397
llvm::errs() << "RequirementMachine generic signature minimization is broken:\n";
8395-
llvm::errs() << "RequirementMachine says: " << rqmResult.getPointer() << "\n";
8396-
llvm::errs() << "GenericSignatureBuilder says: " << gsbResult.getPointer() << "\n";
8398+
llvm::errs() << "RequirementMachine says: ";
8399+
rqmResult.getPointer()->print(llvm::errs(), opts);
8400+
llvm::errs() << "\n";
8401+
llvm::errs() << "GenericSignatureBuilder says: ";
8402+
gsbResult.getPointer()->print(llvm::errs(), opts);
8403+
llvm::errs() << "\n";
83978404

83988405
abort();
83998406
}
@@ -8561,9 +8568,16 @@ InferredGenericSignatureRequest::evaluate(
85618568
return rqmResult;
85628569

85638570
if (!rqmResult.getPointer()->isEqual(gsbResult.getPointer())) {
8571+
PrintOptions opts;
8572+
opts.ProtocolQualifiedDependentMemberTypes = true;
8573+
85648574
llvm::errs() << "RequirementMachine generic signature minimization is broken:\n";
8565-
llvm::errs() << "RequirementMachine says: " << rqmResult.getPointer() << "\n";
8566-
llvm::errs() << "GenericSignatureBuilder says: " << gsbResult.getPointer() << "\n";
8575+
llvm::errs() << "RequirementMachine says: ";
8576+
rqmResult.getPointer()->print(llvm::errs(), opts);
8577+
llvm::errs() << "\n";
8578+
llvm::errs() << "GenericSignatureBuilder says: ";
8579+
gsbResult.getPointer()->print(llvm::errs(), opts);
8580+
llvm::errs() << "\n";
85678581

85688582
abort();
85698583
}
@@ -8663,16 +8677,23 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
86638677
auto gsbResult = buildViaGSB();
86648678

86658679
if (!compare(rqmResult, gsbResult)) {
8680+
PrintOptions opts;
8681+
opts.ProtocolQualifiedDependentMemberTypes = true;
8682+
86668683
llvm::errs() << "RequirementMachine protocol signature minimization is broken:\n";
86678684
llvm::errs() << "Protocol: " << proto->getName() << "\n";
86688685

8686+
llvm::errs() << "RequirementMachine says: ";
86698687
auto rqmSig = GenericSignature::get(
86708688
proto->getGenericSignature().getGenericParams(), rqmResult);
8671-
llvm::errs() << "RequirementMachine says: " << rqmSig << "\n";
8689+
rqmSig.print(llvm::errs(), opts);
8690+
llvm::errs() << "\n";
86728691

8692+
llvm::errs() << "GenericSignatureBuilder says: ";
86738693
auto gsbSig = GenericSignature::get(
86748694
proto->getGenericSignature().getGenericParams(), gsbResult);
8675-
llvm::errs() << "GenericSignatureBuilder says: " << gsbSig << "\n";
8695+
gsbSig.print(llvm::errs(), opts);
8696+
llvm::errs() << "\n";
86768697

86778698
abort();
86788699
}

‎lib/AST/RequirementMachine/RequirementLowering.cpp

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,18 @@ static void desugarLayoutRequirement(Type subjectType,
116116
result.emplace_back(RequirementKind::Layout, subjectType, layout);
117117
}
118118

119+
static Type lookupMemberType(Type subjectType, ProtocolDecl *protoDecl,
120+
AssociatedTypeDecl *assocType) {
121+
if (subjectType->isTypeParameter())
122+
return DependentMemberType::get(subjectType, assocType);
123+
124+
auto *M = protoDecl->getParentModule();
125+
auto conformance = M->lookupConformance(
126+
subjectType, protoDecl);
127+
return conformance.getAssociatedType(subjectType,
128+
assocType->getDeclaredInterfaceType());
129+
}
130+
119131
/// Desugar a protocol conformance requirement by splitting up protocol
120132
/// compositions on the right hand side into conformance and superclass
121133
/// requirements.
@@ -124,7 +136,23 @@ static void desugarConformanceRequirement(Type subjectType, Type constraintType,
124136
// Fast path.
125137
if (constraintType->is<ProtocolType>()) {
126138
if (!subjectType->isTypeParameter()) {
127-
// FIXME: Check conformance, diagnose redundancy or conflict upstream
139+
// Check if the subject type actually conforms.
140+
auto *protoDecl = constraintType->castTo<ProtocolType>()->getDecl();
141+
auto *module = protoDecl->getParentModule();
142+
auto conformance = module->lookupConformance(subjectType, protoDecl);
143+
if (conformance.isInvalid()) {
144+
// FIXME: Diagnose a conflict.
145+
return;
146+
}
147+
148+
// FIXME: Diagnose a redundancy.
149+
assert(conformance.isConcrete());
150+
auto *concrete = conformance.getConcrete();
151+
152+
// Introduce conditional requirements if the subject type is concrete.
153+
for (auto req : concrete->getConditionalRequirements()) {
154+
desugarRequirement(req, result);
155+
}
128156
return;
129157
}
130158

@@ -141,16 +169,7 @@ static void desugarConformanceRequirement(Type subjectType, Type constraintType,
141169

142170
auto *assocType = protoDecl->getPrimaryAssociatedType();
143171

144-
Type memberType;
145-
if (!subjectType->isTypeParameter()) {
146-
auto *M = protoDecl->getParentModule();
147-
auto conformance = M->lookupConformance(
148-
subjectType, protoDecl);
149-
memberType = conformance.getConcrete()->getTypeWitness(assocType);
150-
} else {
151-
memberType = DependentMemberType::get(subjectType, assocType);
152-
}
153-
172+
auto memberType = lookupMemberType(subjectType, protoDecl, assocType);
154173
desugarSameTypeRequirement(memberType, paramType->getArgumentType(),
155174
result);
156175
return;
@@ -278,10 +297,7 @@ struct InferRequirementsWalker : public TypeWalker {
278297
auto addSameTypeConstraint = [&](Type firstType,
279298
AssociatedTypeDecl *assocType) {
280299
auto *protocol = assocType->getProtocol();
281-
auto *module = protocol->getParentModule();
282-
auto conf = module->lookupConformance(firstType, protocol);
283-
auto secondType = conf.getAssociatedType(
284-
firstType, assocType->getDeclaredInterfaceType());
300+
auto secondType = lookupMemberType(firstType, protocol, assocType);
285301
Requirement req(RequirementKind::SameType, firstType, secondType);
286302
desugarRequirement(req, reqs);
287303
};

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,41 @@ RequirementMachine::buildRequirementsFromRules(
123123
return;
124124

125125
case Symbol::Kind::Superclass: {
126-
// For compatibility with the old GenericSignatureBuilder, drop requirements
127-
// containing ErrorTypes.
126+
// Requirements containing unresolved name symbols originate from
127+
// invalid code and should not appear in the generic signature.
128+
for (auto term : prop->getSubstitutions()) {
129+
if (term.containsUnresolvedSymbols())
130+
return;
131+
}
132+
133+
// Requirements containing error types originate from invalid code
134+
// and should not appear in the generic signature.
135+
if (prop->getConcreteType()->hasError())
136+
return;
137+
128138
auto superclassType = Map.getTypeFromSubstitutionSchema(
129139
prop->getConcreteType(),
130140
prop->getSubstitutions(),
131141
genericParams, MutableTerm());
132-
if (superclassType->hasError())
133-
return;
134142

135143
reqs.emplace_back(RequirementKind::Superclass,
136144
subjectType, superclassType);
137145
return;
138146
}
139147

140148
case Symbol::Kind::ConcreteType: {
149+
// Requirements containing unresolved name symbols originate from
150+
// invalid code and should not appear in the generic signature.
151+
for (auto term : prop->getSubstitutions()) {
152+
if (term.containsUnresolvedSymbols())
153+
return;
154+
}
155+
156+
// Requirements containing error types originate from invalid code
157+
// and should not appear in the generic signature.
158+
if (prop->getConcreteType()->hasError())
159+
return;
160+
141161
auto concreteType = Map.getTypeFromSubstitutionSchema(
142162
prop->getConcreteType(),
143163
prop->getSubstitutions(),
@@ -465,12 +485,14 @@ InferredGenericSignatureRequestRQM::evaluate(
465485
requirements);
466486
}
467487

488+
auto *lookupDC = (*gpList->begin())->getDeclContext();
489+
468490
// Add the generic parameter list's 'where' clause to the builder.
469491
//
470492
// The only time generic parameter lists have a 'where' clause is
471493
// in SIL mode; all other generic declarations have a free-standing
472494
// 'where' clause, which will be visited below.
473-
WhereClauseOwner(parentModule, gpList)
495+
WhereClauseOwner(lookupDC, gpList)
474496
.visitRequirements(TypeResolutionStage::Structural,
475497
visitRequirement);
476498
}

‎lib/AST/TypeCheckRequests.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,9 @@ SourceLoc WhereClauseOwner::getLoc() const {
356356
if (auto attr = source.dyn_cast<SpecializeAttr *>())
357357
return attr->getLocation();
358358

359+
if (auto attr = source.dyn_cast<DifferentiableAttr *>())
360+
return attr->getLocation();
361+
359362
return source.get<GenericParamList *>()->getWhereLoc();
360363
}
361364

@@ -365,6 +368,8 @@ void swift::simple_display(llvm::raw_ostream &out,
365368
simple_display(out, owner.dc->getAsDecl());
366369
} else if (owner.source.is<SpecializeAttr *>()) {
367370
out << "@_specialize";
371+
} else if (owner.source.is<DifferentiableAttr *>()) {
372+
out << "@_differentiable";
368373
} else {
369374
out << "(SIL generic parameter list)";
370375
}

‎lib/Sema/TypeCheckType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
679679

680680
generic->setInvalid();
681681
}
682-
return type;
682+
return ErrorType::get(ctx);
683683
}
684684

685685
auto *unboundType = type->castTo<UnboundGenericType>();

‎test/Generics/conditional_requirement_inference_2.swift

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ protocol Sequence {
1515
extension Array : Sequence {}
1616

1717
struct EquatableSequenceBox<T : Sequence> where T.Element : Equatable {
18-
// CHECK: Generic signature: <T, U where T == Array<Array<U>>, U : Equatable>
18+
// CHECK-LABEL: EquatableSequenceBox.withArrayArray@
19+
// CHECK-NEXT: Generic signature: <T, U where T == Array<Array<U>>, U : Equatable>
1920
func withArrayArray<U>(_: U) where T == Array<Array<U>> {}
20-
}
21+
}
22+
23+
// Make sure requirement desugaring handles conditional conformances.
24+
protocol Hashable : Equatable {}
25+
26+
struct Set<Element : Hashable> {}
27+
28+
extension Array : Hashable where Element : Hashable {}
29+
30+
// CHECK-LABEL: doStuff@
31+
// CHECK-NEXT: Generic signature: <U where U : Hashable>
32+
func doStuff<U>(_: Set<Array<U>>) {}

‎test/Generics/rdar28544316.swift

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %target-swift-frontend %s -emit-ir -debug-generic-signatures -requirement-machine-inferred-signatures=on 2>&1 | %FileCheck %s
2+
// REQUIRES: asserts
3+
4+
class PropertyDataSource<O: PropertyHosting> {
5+
}
6+
7+
protocol TableViewCellFactoryType {
8+
associatedtype Item
9+
}
10+
11+
public protocol PropertyHosting {
12+
associatedtype PType: Hashable, EntityOwned
13+
}
14+
15+
public protocol EntityOwned: class {
16+
associatedtype Owner
17+
}
18+
19+
public protocol PropertyType: class {
20+
}
21+
22+
func useType<T>(cellType: T.Type) {
23+
}
24+
25+
// The GSB would reject this declaration because it was "too minimal",
26+
// missing the Factory.Item : EntityOwned requirement that is
27+
// required to get a conformance-valid rewrite system.
28+
//
29+
// The Requirement Machine correctly infers this requirement and adds
30+
// it during minimization.
31+
32+
// CHECK-LABEL: rdar28544316.(file).PropertyTableViewAdapter@
33+
// CHECK-NEXT: Generic signature: <Factory where Factory : TableViewCellFactoryType, Factory.[TableViewCellFactoryType]Item : EntityOwned, Factory.[TableViewCellFactoryType]Item : PropertyType, Factory.[TableViewCellFactoryType]Item == Factory.[TableViewCellFactoryType]Item.[EntityOwned]Owner.[PropertyHosting]PType, Factory.[TableViewCellFactoryType]Item.[EntityOwned]Owner : PropertyHosting>
34+
35+
final class PropertyTableViewAdapter<Factory: TableViewCellFactoryType>
36+
where
37+
Factory.Item: PropertyType,
38+
Factory.Item.Owner: PropertyHosting,
39+
Factory.Item.Owner.PType == Factory.Item
40+
{
41+
typealias Item = Factory.Item
42+
43+
let dataManager: PropertyDataSource<Factory.Item.Owner>
44+
init(dataManager: PropertyDataSource<Factory.Item.Owner>) {
45+
useType(cellType: Factory.Item.Owner.self)
46+
self.dataManager = dataManager
47+
}
48+
}

‎validation-test/compiler_crashers_2_fixed/0074-rdar28544316.swift

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)