Skip to content

Commit f5b3d19

Browse files
slavapestovslavapestov
committed
RequirementMachine: Replace hadError() with getErrors() returning an OptionSet
This gives us more fine-grained information which will be plumbed through the various requests.
1 parent 1578ab5 commit f5b3d19

File tree

9 files changed

+78
-48
lines changed

9 files changed

+78
-48
lines changed

‎include/swift/AST/GenericSignature.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,29 @@ GenericSignature buildGenericSignature(
529529
SmallVector<GenericTypeParamType *, 2> addedParameters,
530530
SmallVector<Requirement, 2> addedRequirements);
531531

532+
/// Summary of error conditions detected by the Requirement Machine.
533+
enum class GenericSignatureErrorFlags {
534+
/// The original requirements referenced a non-existent type parameter.
535+
HasUnresolvedType = (1<<0),
536+
537+
/// The original requirements were in conflict with each other, meaning
538+
/// there are no possible concrete substitutions which statisfy the
539+
/// generic signature.
540+
HasConflict = (1<<1),
541+
542+
/// The Knuth-Bendix completion procedure failed to construct a confluent
543+
/// rewrite system.
544+
CompletionFailed = (1<<2)
545+
};
546+
547+
using GenericSignatureErrors = OptionSet<GenericSignatureErrorFlags>;
548+
549+
/// AbstractGenericSignatureRequest and InferredGenericSignatureRequest
550+
/// return this type, which stores a GenericSignature together with the
551+
/// above set of error flags.
552+
using GenericSignatureWithError = llvm::PointerIntPair<GenericSignature, 3,
553+
GenericSignatureErrors>;
554+
532555
} // end namespace swift
533556

534557
namespace llvm {

‎include/swift/AST/RequirementSignature.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#ifndef SWIFT_AST_REQUIREMENT_SIGNATURE_H
1818
#define SWIFT_AST_REQUIREMENT_SIGNATURE_H
1919

20+
#include "swift/AST/GenericSignature.h"
2021
#include "swift/AST/Type.h"
2122

2223
namespace swift {
@@ -42,13 +43,16 @@ class ProtocolTypeAlias final {
4243
class RequirementSignature final {
4344
ArrayRef<Requirement> Requirements;
4445
ArrayRef<ProtocolTypeAlias> TypeAliases;
46+
GenericSignatureErrors Errors;
4547

4648
public:
47-
RequirementSignature() = default;
49+
RequirementSignature(GenericSignatureErrors errors = GenericSignatureErrors())
50+
: Errors(errors) {}
4851

4952
RequirementSignature(ArrayRef<Requirement> requirements,
50-
ArrayRef<ProtocolTypeAlias> typeAliases)
51-
: Requirements(requirements), TypeAliases(typeAliases) {}
53+
ArrayRef<ProtocolTypeAlias> typeAliases,
54+
GenericSignatureErrors errors = GenericSignatureErrors())
55+
: Requirements(requirements), TypeAliases(typeAliases), Errors(errors) {}
5256

5357
/// The requirements including any inherited protocols and conformances for
5458
/// associated types that are introduced in this protocol.
@@ -65,6 +69,10 @@ class RequirementSignature final {
6569
ArrayRef<ProtocolTypeAlias> getTypeAliases() const {
6670
return TypeAliases;
6771
}
72+
73+
GenericSignatureErrors getErrors() const {
74+
return Errors;
75+
}
6876
};
6977

7078
} // end namespace swift

‎include/swift/AST/TypeCheckRequests.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1747,12 +1747,6 @@ class ClassAncestryFlagsRequest :
17471747

17481748
void simple_display(llvm::raw_ostream &out, AncestryFlags value);
17491749

1750-
/// AbstractGenericSignatureRequest and InferredGenericSignatureRequest
1751-
/// return this type, which stores a GenericSignature together with a bit
1752-
/// indicating if there were any errors detected in the original
1753-
/// requirements.
1754-
using GenericSignatureWithError = llvm::PointerIntPair<GenericSignature, 1>;
1755-
17561750
class AbstractGenericSignatureRequest :
17571751
public SimpleRequest<AbstractGenericSignatureRequest,
17581752
GenericSignatureWithError (const GenericSignatureImpl *,

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8234,7 +8234,7 @@ AbstractGenericSignatureRequest::evaluate(
82348234
// If nothing is added to the base signature, just return the base
82358235
// signature.
82368236
if (addedParameters.empty() && addedRequirements.empty())
8237-
return GenericSignatureWithError(baseSignature, /*hadError=*/false);
8237+
return GenericSignatureWithError(baseSignature, GenericSignatureErrors());
82388238

82398239
ASTContext &ctx = addedParameters.empty()
82408240
? addedRequirements.front().getFirstType()->getASTContext()
@@ -8328,7 +8328,7 @@ AbstractGenericSignatureRequestGSB::evaluate(
83288328
// If nothing is added to the base signature, just return the base
83298329
// signature.
83308330
if (addedParameters.empty() && addedRequirements.empty())
8331-
return GenericSignatureWithError(baseSignature, /*hadError=*/false);
8331+
return GenericSignatureWithError(baseSignature, GenericSignatureErrors());
83328332

83338333
ASTContext &ctx = addedParameters.empty()
83348334
? addedRequirements.front().getFirstType()->getASTContext()
@@ -8343,7 +8343,7 @@ AbstractGenericSignatureRequestGSB::evaluate(
83438343

83448344
auto result = GenericSignature::get(addedParameters,
83458345
baseSignature.getRequirements());
8346-
return GenericSignatureWithError(result, /*hadError=*/false);
8346+
return GenericSignatureWithError(result, GenericSignatureErrors());
83478347
}
83488348

83498349
// If the request is non-canonical, we won't need to build our own
@@ -8423,10 +8423,12 @@ AbstractGenericSignatureRequestGSB::evaluate(
84238423
for (const auto &req : addedRequirements)
84248424
builder.addRequirement(req, source, nullptr);
84258425

8426-
bool hadError = builder.hadAnyError();
8426+
GenericSignatureErrors errorFlags;
8427+
if (builder.hadAnyError())
8428+
errorFlags |= GenericSignatureErrorFlags::HasUnresolvedType;
84278429
auto result = std::move(builder).computeGenericSignature(
84288430
/*allowConcreteGenericParams=*/true);
8429-
return GenericSignatureWithError(result, hadError);
8431+
return GenericSignatureWithError(result, errorFlags);
84308432
}
84318433

84328434
GenericSignatureWithError
@@ -8627,10 +8629,12 @@ InferredGenericSignatureRequestGSB::evaluate(
86278629
for (const auto &req : addedRequirements)
86288630
builder.addRequirement(req, source, parentModule);
86298631

8630-
bool hadError = builder.hadAnyError();
8632+
GenericSignatureErrors errorFlags;
8633+
if (builder.hadAnyError())
8634+
errorFlags |= GenericSignatureErrorFlags::HasUnresolvedType;
86318635
auto result = std::move(builder).computeGenericSignature(
86328636
allowConcreteGenericParams);
8633-
return GenericSignatureWithError(result, hadError);
8637+
return GenericSignatureWithError(result, errorFlags);
86348638
}
86358639

86368640
RequirementSignature

‎lib/AST/RequirementMachine/HomotopyReduction.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -930,31 +930,31 @@ void RewriteSystem::minimizeRewriteSystem() {
930930
normalizeRedundantRules();
931931
}
932932

933-
/// In a conformance-valid rewrite system, any rule with unresolved symbols on
934-
/// the left or right hand side should be redundant. The presence of unresolved
935-
/// non-redundant rules means one of the original requirements written by the
936-
/// user was invalid.
937-
bool RewriteSystem::hadError() const {
933+
/// Returns flags indicating if the rewrite system has unresolved or
934+
/// conflicting rules in our minimization domain.
935+
GenericSignatureErrors RewriteSystem::getErrors() const {
938936
assert(Complete);
939937
assert(Minimized);
940938

941-
for (const auto &rule : Rules) {
942-
if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
943-
continue;
939+
GenericSignatureErrors result;
944940

941+
for (const auto &rule : Rules) {
945942
if (rule.isPermanent())
946943
continue;
947944

948-
if (rule.isConflicting())
949-
return true;
945+
if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
946+
continue;
950947

951948
if (!rule.isRedundant() &&
952949
!rule.isProtocolTypeAliasRule() &&
953950
rule.containsUnresolvedSymbols())
954-
return true;
951+
result |= GenericSignatureErrorFlags::HasUnresolvedType;
952+
953+
if (rule.isConflicting())
954+
result |= GenericSignatureErrorFlags::HasConflict;
955955
}
956956

957-
return false;
957+
return result;
958958
}
959959

960960
/// Collect all non-permanent, non-redundant rules whose domain is equal to

‎lib/AST/RequirementMachine/RequirementMachine.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,9 @@ bool RequirementMachine::isComplete() const {
294294
return Complete;
295295
}
296296

297-
bool RequirementMachine::hadError() const {
298-
// FIXME: Implement other checks here
299-
// FIXME: Assert if hadError() is true but we didn't emit any diagnostics?
300-
return System.hadError();
297+
GenericSignatureErrors RequirementMachine::getErrors() const {
298+
// FIXME: Assert if we had errors but we didn't emit any diagnostics?
299+
return System.getErrors();
301300
}
302301

303302
void RequirementMachine::dump(llvm::raw_ostream &out) const {

‎lib/AST/RequirementMachine/RequirementMachine.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class RequirementMachine final {
155155

156156
std::string getRuleAsStringForDiagnostics(unsigned ruleID) const;
157157

158-
bool hadError() const;
158+
GenericSignatureErrors getErrors() const;
159159

160160
void verify(const MutableTerm &term) const;
161161
void dump(llvm::raw_ostream &out) const;

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ RequirementMachine::computeMinimalProtocolRequirements() {
7676
reqs, aliases);
7777

7878
result[proto] = RequirementSignature(ctx.AllocateCopy(reqs),
79-
ctx.AllocateCopy(aliases));
79+
ctx.AllocateCopy(aliases),
80+
getErrors());
8081
}
8182

8283
return result;
@@ -133,11 +134,11 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
133134
if (otherProto != proto) {
134135
ctx.evaluator.cacheOutput(
135136
RequirementSignatureRequestRQM{const_cast<ProtocolDecl *>(otherProto)},
136-
RequirementSignature());
137+
RequirementSignature(GenericSignatureErrorFlags::CompletionFailed));
137138
}
138139
}
139140

140-
return RequirementSignature();
141+
return RequirementSignature(GenericSignatureErrorFlags::CompletionFailed);
141142
}
142143

143144
auto minimalRequirements = machine->computeMinimalProtocolRequirements();
@@ -252,7 +253,7 @@ AbstractGenericSignatureRequestRQM::evaluate(
252253
// If nothing is added to the base signature, just return the base
253254
// signature.
254255
if (addedParameters.empty() && addedRequirements.empty())
255-
return GenericSignatureWithError(baseSignature, /*hadError=*/false);
256+
return GenericSignatureWithError(baseSignature, GenericSignatureErrors());
256257

257258
ASTContext &ctx = addedParameters.empty()
258259
? addedRequirements.front().getFirstType()->getASTContext()
@@ -270,7 +271,7 @@ AbstractGenericSignatureRequestRQM::evaluate(
270271
if (addedRequirements.empty()) {
271272
auto result = GenericSignature::get(genericParams,
272273
baseSignature.getRequirements());
273-
return GenericSignatureWithError(result, /*hadError=*/false);
274+
return GenericSignatureWithError(result, GenericSignatureErrors());
274275
}
275276

276277
// If the request is non-canonical, we won't need to build our own
@@ -388,12 +389,12 @@ AbstractGenericSignatureRequestRQM::evaluate(
388389
/*reconstituteSugar=*/false);
389390

390391
auto result = GenericSignature::get(genericParams, minimalRequirements);
391-
bool hadError = machine->hadError();
392+
auto errorFlags = machine->getErrors();
392393

393-
if (!hadError)
394+
if (!errorFlags)
394395
result.verify();
395396

396-
return GenericSignatureWithError(result, hadError);
397+
return GenericSignatureWithError(result, errorFlags);
397398
}
398399

399400
GenericSignatureWithError
@@ -522,28 +523,28 @@ InferredGenericSignatureRequestRQM::evaluate(
522523
rule);
523524

524525
auto result = GenericSignature::get(genericParams, {});
525-
return GenericSignatureWithError(result, /*hadError=*/true);
526+
return GenericSignatureWithError(
527+
result, GenericSignatureErrorFlags::CompletionFailed);
526528
}
527529

528530
auto minimalRequirements =
529531
machine->computeMinimalGenericSignatureRequirements(
530532
/*reconstituteSugar=*/true);
531533

532534
auto result = GenericSignature::get(genericParams, minimalRequirements);
533-
bool hadError = machine->hadError();
535+
auto errorFlags = machine->getErrors();
534536

535537
if (ctx.LangOpts.RequirementMachineInferredSignatures ==
536538
RequirementMachineMode::Enabled) {
537539
machine->System.computeRedundantRequirementDiagnostics(errors);
538-
hadError |= diagnoseRequirementErrors(ctx, errors,
539-
allowConcreteGenericParams);
540+
diagnoseRequirementErrors(ctx, errors, allowConcreteGenericParams);
540541
}
541542

542543
// FIXME: Handle allowConcreteGenericParams
543544

544545
// Check invariants.
545-
if (!hadError)
546+
if (!errorFlags)
546547
result.verify();
547548

548-
return GenericSignatureWithError(result, hadError);
549+
return GenericSignatureWithError(result, errorFlags);
549550
}

‎lib/AST/RequirementMachine/RewriteSystem.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define SWIFT_REWRITESYSTEM_H
1515

1616
#include "swift/AST/Requirement.h"
17+
#include "swift/AST/TypeCheckRequests.h"
1718
#include "llvm/ADT/DenseSet.h"
1819
#include "llvm/ADT/PointerUnion.h"
1920

@@ -537,7 +538,7 @@ class RewriteSystem final {
537538

538539
void minimizeRewriteSystem();
539540

540-
bool hadError() const;
541+
GenericSignatureErrors getErrors() const;
541542

542543
struct MinimizedProtocolRules {
543544
std::vector<unsigned> Requirements;

0 commit comments

Comments
 (0)