Skip to content

Commit 4b55c30

Browse files
hborlahborla
committed
[RequirementMachine] Instead of recording redundant requirements and other
diagnostics in the rewrite system, pass down the 'errors' vector from the top-level requests.
1 parent b5d7a01 commit 4b55c30

File tree

7 files changed

+46
-57
lines changed

7 files changed

+46
-57
lines changed

‎lib/AST/RequirementMachine/ConcreteTypeWitness.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ void PropertyMap::inferConditionalRequirements(
581581
for (const auto &rule : builder.RequirementRules) {
582582
auto lhs = std::get<0>(rule);
583583
auto rhs = std::get<1>(rule);
584-
System.addExplicitRule(lhs, rhs, /*requirementID=*/None);
584+
System.addExplicitRule(lhs, rhs, /*requirementID=*/None, errors);
585585
}
586586
}
587587
}

‎lib/AST/RequirementMachine/RequirementMachine.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ void RequirementMachine::checkCompletionResult(CompletionResult result) const {
7575
///
7676
/// Returns failure if completion fails within the configured number of steps.
7777
std::pair<CompletionResult, unsigned>
78-
RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
78+
RequirementMachine::initWithGenericSignature(CanGenericSignature sig,
79+
SmallVectorImpl<RequirementError> &errors) {
7980
Sig = sig;
8081
Params.append(sig.getGenericParams().begin(),
8182
sig.getGenericParams().end());
@@ -98,7 +99,8 @@ RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
9899
/*protos=*/ArrayRef<const ProtocolDecl *>(),
99100
std::move(builder.WrittenRequirements),
100101
std::move(builder.PermanentRules),
101-
std::move(builder.RequirementRules));
102+
std::move(builder.RequirementRules),
103+
errors);
102104

103105
auto result = computeCompletion(RewriteSystem::DisallowInvalidRequirements);
104106

@@ -123,7 +125,8 @@ RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
123125
///
124126
/// Returns failure if completion fails within the configured number of steps.
125127
std::pair<CompletionResult, unsigned>
126-
RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
128+
RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
129+
SmallVectorImpl<RequirementError> &errors) {
127130
FrontendStatsTracer tracer(Stats, "build-rewrite-system");
128131

129132
if (Dump) {
@@ -141,7 +144,8 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
141144
System.initialize(/*recordLoops=*/true, protos,
142145
std::move(builder.WrittenRequirements),
143146
std::move(builder.PermanentRules),
144-
std::move(builder.RequirementRules));
147+
std::move(builder.RequirementRules),
148+
errors);
145149

146150
auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
147151

@@ -167,7 +171,8 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
167171
std::pair<CompletionResult, unsigned>
168172
RequirementMachine::initWithWrittenRequirements(
169173
ArrayRef<GenericTypeParamType *> genericParams,
170-
ArrayRef<StructuralRequirement> requirements) {
174+
ArrayRef<StructuralRequirement> requirements,
175+
SmallVectorImpl<RequirementError> &errors) {
171176
Params.append(genericParams.begin(), genericParams.end());
172177

173178
FrontendStatsTracer tracer(Stats, "build-rewrite-system");
@@ -189,7 +194,8 @@ RequirementMachine::initWithWrittenRequirements(
189194
/*protos=*/ArrayRef<const ProtocolDecl *>(),
190195
std::move(builder.WrittenRequirements),
191196
std::move(builder.PermanentRules),
192-
std::move(builder.RequirementRules));
197+
std::move(builder.RequirementRules),
198+
errors);
193199

194200
auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
195201

‎lib/AST/RequirementMachine/RequirementMachine.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,18 @@ class RequirementMachine final {
9191
void checkCompletionResult(CompletionResult result) const;
9292

9393
std::pair<CompletionResult, unsigned>
94-
initWithGenericSignature(CanGenericSignature sig);
94+
initWithGenericSignature(CanGenericSignature sig,
95+
SmallVectorImpl<RequirementError> &errors);
9596

9697
std::pair<CompletionResult, unsigned>
97-
initWithProtocols(ArrayRef<const ProtocolDecl *> protos);
98+
initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
99+
SmallVectorImpl<RequirementError> &errors);
98100

99101
std::pair<CompletionResult, unsigned>
100102
initWithWrittenRequirements(
101103
ArrayRef<GenericTypeParamType *> genericParams,
102-
ArrayRef<StructuralRequirement> requirements);
104+
ArrayRef<StructuralRequirement> requirements,
105+
SmallVectorImpl<RequirementError> &errors);
103106

104107
bool isComplete() const;
105108

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,9 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
352352
std::unique_ptr<RequirementMachine> machine(new RequirementMachine(
353353
ctx.getRewriteContext()));
354354

355-
auto status = machine->initWithProtocols(component);
355+
SmallVector<RequirementError, 4> errors;
356+
357+
auto status = machine->initWithProtocols(component, errors);
356358
if (status.first != CompletionResult::Success) {
357359
// All we can do at this point is diagnose and give each protocol an empty
358360
// requirement signature.
@@ -421,15 +423,8 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
421423

422424
if (ctx.LangOpts.RequirementMachineProtocolSignatures ==
423425
RequirementMachineMode::Enabled) {
424-
// Diagnose trivially invalid requirements from the rewrite
425-
// system.
426-
diagnoseRequirementErrors(ctx, machine->System.getErrors(),
427-
/*allowConcreteGenericParams=*/false);
428-
429-
// Diagnose redundant requirements found during signature
430-
// minimization.
431-
auto redundancies = machine->System.getRedundantRequirements();
432-
diagnoseRequirementErrors(ctx, redundancies,
426+
machine->System.computeRedundantRequirementDiagnostics(errors);
427+
diagnoseRequirementErrors(ctx, errors,
433428
/*allowConcreteGenericParams=*/false);
434429
}
435430

@@ -529,7 +524,7 @@ AbstractGenericSignatureRequestRQM::evaluate(
529524
ctx.getRewriteContext()));
530525

531526
auto status =
532-
machine->initWithWrittenRequirements(genericParams, requirements);
527+
machine->initWithWrittenRequirements(genericParams, requirements, errors);
533528
machine->checkCompletionResult(status.first);
534529

535530
auto minimalRequirements =
@@ -657,7 +652,7 @@ InferredGenericSignatureRequestRQM::evaluate(
657652
ctx.getRewriteContext()));
658653

659654
auto status =
660-
machine->initWithWrittenRequirements(genericParams, requirements);
655+
machine->initWithWrittenRequirements(genericParams, requirements, errors);
661656
if (status.first != CompletionResult::Success) {
662657
ctx.Diags.diagnose(loc,
663658
diag::requirement_machine_completion_failed,
@@ -681,20 +676,9 @@ InferredGenericSignatureRequestRQM::evaluate(
681676

682677
if (ctx.LangOpts.RequirementMachineInferredSignatures ==
683678
RequirementMachineMode::Enabled) {
684-
// Diagnose invalid requirements dropped during desugaring.
679+
machine->System.computeRedundantRequirementDiagnostics(errors);
685680
hadError |= diagnoseRequirementErrors(ctx, errors,
686681
allowConcreteGenericParams);
687-
688-
// Diagnose trivially invalid requirements from the rewrite
689-
// system.
690-
hadError |= diagnoseRequirementErrors(ctx, machine->System.getErrors(),
691-
allowConcreteGenericParams);
692-
693-
// Diagnose redundant requirements found during signature
694-
// minimization.
695-
auto redundancies = machine->System.getRedundantRequirements();
696-
hadError |= diagnoseRequirementErrors(ctx, redundancies,
697-
allowConcreteGenericParams);
698682
}
699683

700684
// FIXME: Handle allowConcreteGenericParams

‎lib/AST/RequirementMachine/RewriteContext.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,11 @@ RequirementMachine *RewriteContext::getRequirementMachine(
133133
auto *newMachine = new rewriting::RequirementMachine(*this);
134134
machine = newMachine;
135135

136+
SmallVector<RequirementError, 4> errors;
137+
136138
// This might re-entrantly invalidate 'machine', which is a reference
137139
// into Protos.
138-
auto status = newMachine->initWithGenericSignature(sig);
140+
auto status = newMachine->initWithGenericSignature(sig, errors);
139141
newMachine->checkCompletionResult(status.first);
140142

141143
return newMachine;

‎lib/AST/RequirementMachine/RewriteSystem.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ void RewriteSystem::initialize(
279279
ArrayRef<StructuralRequirement> writtenRequirements,
280280
std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
281281
std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>>
282-
&&requirementRules) {
282+
&&requirementRules,
283+
SmallVectorImpl<RequirementError> &errors) {
283284
assert(!Initialized);
284285
Initialized = 1;
285286

@@ -294,7 +295,7 @@ void RewriteSystem::initialize(
294295
auto lhs = std::get<0>(rule);
295296
auto rhs = std::get<1>(rule);
296297
auto requirementID = std::get<2>(rule);
297-
addExplicitRule(lhs, rhs, requirementID);
298+
addExplicitRule(lhs, rhs, requirementID, errors);
298299
}
299300
}
300301

@@ -495,13 +496,14 @@ bool RewriteSystem::addPermanentRule(MutableTerm lhs, MutableTerm rhs) {
495496

496497
/// Add a new rule, marking it explicit.
497498
bool RewriteSystem::addExplicitRule(MutableTerm lhs, MutableTerm rhs,
498-
Optional<unsigned> requirementID) {
499+
Optional<unsigned> requirementID,
500+
SmallVectorImpl<RequirementError> &errors) {
499501
bool added = addRule(std::move(lhs), std::move(rhs));
500502
if (added) {
501503
Rules.back().markExplicit(requirementID);
502504
} else if (requirementID.hasValue()) {
503505
auto req = WrittenRequirements[requirementID.getValue()];
504-
Errors.push_back(
506+
errors.push_back(
505507
RequirementError::forRedundantRequirement(req.req, req.loc));
506508
}
507509

@@ -720,11 +722,10 @@ void RewriteSystem::verifyRewriteRules(ValidityPolicy policy) const {
720722
#undef ASSERT_RULE
721723
}
722724

723-
std::vector<RequirementError>
724-
RewriteSystem::getRedundantRequirements() {
725-
// The resulting set of redundant requirements.
726-
std::vector<RequirementError> redundantRequirements;
727-
725+
/// Computes the set of explicit redundant requirements to
726+
/// emit warnings for in the source code.
727+
void RewriteSystem::computeRedundantRequirementDiagnostics(
728+
SmallVectorImpl<RequirementError> &errors) {
728729
// Map redundant rule IDs to their rewrite path for easy access
729730
// in the `isRedundantRule` lambda.
730731
llvm::SmallDenseMap<unsigned, RewritePath> redundantRules;
@@ -792,13 +793,11 @@ RewriteSystem::getRedundantRequirements() {
792793
// then the requirement is unnecessary in the source code.
793794
if (llvm::all_of(ruleIDs, isRedundantRule)) {
794795
auto requirement = WrittenRequirements[requirementID];
795-
redundantRequirements.push_back(
796+
errors.push_back(
796797
RequirementError::forRedundantRequirement(requirement.req,
797798
requirement.loc));
798799
}
799800
}
800-
801-
return redundantRequirements;
802801
}
803802

804803
void RewriteSystem::dump(llvm::raw_ostream &out) const {

‎lib/AST/RequirementMachine/RewriteSystem.h

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,6 @@ class RewriteSystem final {
242242
/// The requirements written in source code.
243243
std::vector<StructuralRequirement> WrittenRequirements;
244244

245-
/// The invalid requirements.
246-
std::vector<RequirementError> Errors;
247-
248245
/// The rules added so far, including rules from our client, as well
249246
/// as rules introduced by the completion procedure.
250247
std::vector<Rule> Rules;
@@ -303,7 +300,8 @@ class RewriteSystem final {
303300
void initialize(bool recordLoops, ArrayRef<const ProtocolDecl *> protos,
304301
ArrayRef<StructuralRequirement> writtenRequirements,
305302
std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
306-
std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>> &&requirementRules);
303+
std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>> &&requirementRules,
304+
SmallVectorImpl<RequirementError> &errors);
307305

308306
ArrayRef<const ProtocolDecl *> getProtocols() const {
309307
return Protos;
@@ -336,7 +334,8 @@ class RewriteSystem final {
336334
bool addPermanentRule(MutableTerm lhs, MutableTerm rhs);
337335

338336
bool addExplicitRule(MutableTerm lhs, MutableTerm rhs,
339-
Optional<unsigned> requirementID);
337+
Optional<unsigned> requirementID,
338+
SmallVectorImpl<RequirementError> &errors);
340339

341340
bool simplify(MutableTerm &term, RewritePath *path=nullptr) const;
342341

@@ -376,11 +375,7 @@ class RewriteSystem final {
376375
///
377376
//////////////////////////////////////////////////////////////////////////////
378377

379-
ArrayRef<RequirementError> getErrors() {
380-
return Errors;
381-
}
382-
383-
std::vector<RequirementError> getRedundantRequirements();
378+
void computeRedundantRequirementDiagnostics(SmallVectorImpl<RequirementError> &errors);
384379

385380
private:
386381
struct CriticalPair {

0 commit comments

Comments
 (0)