Skip to content

RequirementMachine: Fix -verify-generic-signatures mode to not violate invariants #41592

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
37 changes: 35 additions & 2 deletions lib/AST/GenericSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,39 @@ void GenericSignature::verify(ArrayRef<Requirement> reqts) const {
}
}

static Type stripBoundDependentMemberTypes(Type t) {
if (auto *depMemTy = t->getAs<DependentMemberType>()) {
return DependentMemberType::get(
stripBoundDependentMemberTypes(depMemTy->getBase()),
depMemTy->getName());
}

return t;
}

static Requirement stripBoundDependentMemberTypes(Requirement req) {
auto subjectType = stripBoundDependentMemberTypes(req.getFirstType());

switch (req.getKind()) {
case RequirementKind::Conformance:
return Requirement(RequirementKind::Conformance, subjectType,
req.getSecondType());

case RequirementKind::Superclass:
case RequirementKind::SameType:
return Requirement(req.getKind(), subjectType,
req.getSecondType().transform([](Type t) {
return stripBoundDependentMemberTypes(t);
}));

case RequirementKind::Layout:
return Requirement(RequirementKind::Layout, subjectType,
req.getLayoutConstraint());
}

llvm_unreachable("Bad requirement kind");
}

void swift::validateGenericSignature(ASTContext &context,
GenericSignature sig) {
llvm::errs() << "Validating generic signature: ";
Expand All @@ -1007,7 +1040,7 @@ void swift::validateGenericSignature(ASTContext &context,

SmallVector<Requirement, 2> requirements;
for (auto requirement : sig.getRequirements())
requirements.push_back(requirement);
requirements.push_back(stripBoundDependentMemberTypes(requirement));

{
PrettyStackTraceGenericSignature debugStack("verifying", sig);
Expand Down Expand Up @@ -1043,7 +1076,7 @@ void swift::validateGenericSignature(ASTContext &context,
SmallVector<Requirement, 2> newRequirements;
for (unsigned i : indices(requirements)) {
if (i != victimIndex)
newRequirements.push_back(requirements[i]);
newRequirements.push_back(stripBoundDependentMemberTypes(requirements[i]));
}

auto newSigWithError = evaluateOrDefault(
Expand Down
7 changes: 6 additions & 1 deletion lib/AST/RequirementMachine/HomotopyReduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,12 +775,17 @@ void RewriteSystem::minimizeRewriteSystem() {
}

/// In a conformance-valid rewrite system, any rule with unresolved symbols on
/// the left or right hand side should have been simplified by another rule.
/// the left or right hand side should be redundant. The presence of unresolved
/// non-redundant rules means one of the original requirements written by the
/// user was invalid.
bool RewriteSystem::hadError() const {
assert(Complete);
assert(Minimized);

for (const auto &rule : Rules) {
if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
continue;

if (rule.isPermanent())
continue;

Expand Down
31 changes: 30 additions & 1 deletion lib/AST/RequirementMachine/InterfaceType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,16 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
// Return a sugared GenericTypeParamType if we're given an array of
// sugared types to substitute.
unsigned index = GenericParamKey(genericParam).findIndexIn(genericParams);

if (index == genericParams.size()) {
llvm::errs() << "Invalid generic parameter: " << genericParam << "\n";
llvm::errs() << "Valid generic parameters are";
for (auto *otherParam : genericParams)
llvm::errs() << " " << Type(otherParam);
llvm::errs() << "\n";
abort();
}

result = genericParams[index];
return;
}
Expand Down Expand Up @@ -344,7 +354,14 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
assert(prefix.size() > 0);

auto *props = map.lookUpProperties(prefix.rbegin(), prefix.rend());
assert(props != nullptr);
if (props == nullptr) {
llvm::errs() << "Cannot build interface type for term "
<< MutableTerm(begin, end) << "\n";
llvm::errs() << "Prefix does not conform to any protocols: "
<< prefix << "\n\n";
map.dump(llvm::errs());
abort();
}

// Assert that the associated type's protocol appears among the set
// of protocols that the prefix conforms to.
Expand All @@ -356,6 +373,18 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
#endif

assocType = props->getAssociatedType(symbol.getName());
if (assocType == nullptr) {
llvm::errs() << "Cannot build interface type for term "
<< MutableTerm(begin, end) << "\n";
llvm::errs() << "Prefix term does not not have a nested type named "
<< symbol.getName() << ": "
<< prefix << "\n";
llvm::errs() << "Property map entry: ";
props->dump(llvm::errs());
llvm::errs() << "\n\n";
map.dump(llvm::errs());
abort();
}
}

result = DependentMemberType::get(result, assocType);
Expand Down
6 changes: 4 additions & 2 deletions lib/AST/RequirementMachine/MinimalConformances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,9 @@ void MinimalConformances::collectConformanceRules() {
/// That is, we can choose to eliminate <X>.[P], but not <Y>.[P], or vice
/// versa; but it is never valid to eliminate both.
void MinimalConformances::computeCandidateConformancePaths() {
for (const auto &loop : System.getLoops()) {
for (unsigned loopID : indices(System.getLoops())) {
const auto &loop = System.getLoops()[loopID];

if (loop.isDeleted())
continue;

Expand All @@ -483,7 +485,7 @@ void MinimalConformances::computeCandidateConformancePaths() {
continue;

if (Debug.contains(DebugFlags::MinimalConformances)) {
llvm::dbgs() << "Candidate homotopy generator: ";
llvm::dbgs() << "Candidate homotopy generator: (#" << loopID << ") ";
loop.dump(llvm::dbgs(), System);
llvm::dbgs() << "\n";
}
Expand Down
2 changes: 1 addition & 1 deletion test/Generics/validate_stdlib_generic_signatures.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Verifies that all of the generic signatures in the standard library are
// minimal and canonical.

// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift
// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift -requirement-machine-abstract-signatures=on