Skip to content

GSB: Add some more assertions #39196

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 2 commits into from
Sep 8, 2021
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
3 changes: 3 additions & 0 deletions include/swift/AST/GenericSignature.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ class GenericSignature {
/// (archetypes) that correspond to the interface types in this generic
/// signature.
GenericEnvironment *getGenericEnvironment() const;

/// Check invariants.
void verify() const;
};

/// A reference to a canonical generic signature.
Expand Down
177 changes: 177 additions & 0 deletions lib/AST/GenericSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1539,4 +1539,181 @@ int swift::compareDependentTypes(Type type1, Type type2) {
return result;

return 0;
}

void GenericSignature::verify() const {
auto canSig = getCanonicalSignature();

PrettyStackTraceGenericSignature debugStack("checking", canSig);

auto canonicalRequirements = canSig.getRequirements();

// We collect conformance requirements to check that they're minimal.
llvm::SmallDenseMap<CanType, SmallVector<ProtocolDecl *, 2>, 2> conformances;

// Check that the requirements satisfy certain invariants.
for (unsigned idx : indices(canonicalRequirements)) {
debugStack.setRequirement(idx);

const auto &reqt = canonicalRequirements[idx];

// Left-hand side must be a canonical type parameter.
if (reqt.getKind() != RequirementKind::SameType) {
if (!reqt.getFirstType()->isTypeParameter()) {
llvm::errs() << "Left-hand side must be a type parameter: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}

if (!canSig->isCanonicalTypeInContext(reqt.getFirstType())) {
llvm::errs() << "Left-hand side is not canonical: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
}

// Check canonicalization of requirement itself.
switch (reqt.getKind()) {
case RequirementKind::Superclass:
if (!canSig->isCanonicalTypeInContext(reqt.getSecondType())) {
llvm::errs() << "Right-hand side is not canonical: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
break;

case RequirementKind::Layout:
break;

case RequirementKind::SameType: {
auto isCanonicalAnchor = [&](Type type) {
if (auto *dmt = type->getAs<DependentMemberType>())
return canSig->isCanonicalTypeInContext(dmt->getBase());
return type->is<GenericTypeParamType>();
};

auto firstType = reqt.getFirstType();
auto secondType = reqt.getSecondType();
if (!isCanonicalAnchor(firstType)) {
llvm::errs() << "Left hand side does not have a canonical parent: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}

if (reqt.getSecondType()->isTypeParameter()) {
if (!isCanonicalAnchor(secondType)) {
llvm::errs() << "Right hand side does not have a canonical parent: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
if (compareDependentTypes(firstType, secondType) >= 0) {
llvm::errs() << "Out-of-order type parameters: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
} else {
if (!canSig->isCanonicalTypeInContext(secondType)) {
llvm::errs() << "Right hand side is not canonical: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
}
break;
}

case RequirementKind::Conformance:
// Collect all conformance requirements on each type parameter.
conformances[CanType(reqt.getFirstType())].push_back(
reqt.getProtocolDecl());
break;
}

// From here on, we're only interested in requirements beyond the first.
if (idx == 0) continue;

// Make sure that the left-hand sides are in nondecreasing order.
const auto &prevReqt = canonicalRequirements[idx-1];
int compareLHS =
compareDependentTypes(prevReqt.getFirstType(), reqt.getFirstType());
if (compareLHS > 0) {
llvm::errs() << "Out-of-order left-hand side: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}

// If we have two same-type requirements where the left-hand sides differ
// but fall into the same equivalence class, we can check the form.
if (compareLHS < 0 && reqt.getKind() == RequirementKind::SameType &&
prevReqt.getKind() == RequirementKind::SameType &&
canSig->areSameTypeParameterInContext(prevReqt.getFirstType(),
reqt.getFirstType())) {
// If it's a it's a type parameter, make sure the equivalence class is
// wired together sanely.
if (prevReqt.getSecondType()->isTypeParameter()) {
if (!prevReqt.getSecondType()->isEqual(reqt.getFirstType())) {
llvm::errs() << "Same-type requirement within an equiv. class "
<< "is out-of-order: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
} else {
// Otherwise, the concrete types must match up.
if (!prevReqt.getSecondType()->isEqual(reqt.getSecondType())) {
llvm::errs() << "Inconsistent concrete requirement in equiv. class: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
}
}

// If we have a concrete same-type requirement, we shouldn't have any
// other requirements on the same type.
if (reqt.getKind() == RequirementKind::SameType &&
!reqt.getSecondType()->isTypeParameter()) {
if (compareLHS >= 0) {
llvm::errs() << "Concrete subject type should not have "
<< "any other requirements: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
}

if (prevReqt.compare(reqt) >= 0) {
llvm::errs() << "Out-of-order requirement: ";
reqt.dump(llvm::errs());
llvm::errs() << "\n";
abort();
}
}

// Make sure we don't have redundant protocol conformance requirements.
for (auto pair : conformances) {
const auto &protos = pair.second;
auto canonicalProtos = protos;

// canonicalizeProtocols() will sort them and filter out any protocols that
// are refined by other protocols in the list. It should be a no-op at this
// point.
ProtocolType::canonicalizeProtocols(canonicalProtos);

if (protos.size() != canonicalProtos.size()) {
llvm::errs() << "Redundant conformance requirements in signature\n";
abort();
}
if (!std::equal(protos.begin(), protos.end(), canonicalProtos.begin())) {
llvm::errs() << "Out-of-order conformance requirements\n";
abort();
}
}
}
35 changes: 34 additions & 1 deletion lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7997,6 +7997,9 @@ static void checkGenericSignature(CanGenericSignature canSig,

auto canonicalRequirements = canSig.getRequirements();

// We collect conformance requirements to check that they're minimal.
llvm::SmallDenseMap<CanType, SmallVector<ProtocolDecl *, 2>, 2> conformances;

// Check that the signature is canonical.
for (unsigned idx : indices(canonicalRequirements)) {
debugStack.setRequirement(idx);
Expand Down Expand Up @@ -8047,6 +8050,10 @@ static void checkGenericSignature(CanGenericSignature canSig,
"Left-hand side must be a type parameter");
assert(isa<ProtocolType>(reqt.getSecondType().getPointer()) &&
"Right-hand side of conformance isn't a protocol type");

// Collect all conformance requirements on each type parameter.
conformances[CanType(reqt.getFirstType())].push_back(
reqt.getProtocolDecl());
break;
}

Expand Down Expand Up @@ -8089,6 +8096,22 @@ static void checkGenericSignature(CanGenericSignature canSig,
assert(prevReqt.compare(reqt) < 0 &&
"Out-of-order requirements");
}

// Make sure we don't have redundant protocol conformance requirements.
for (auto pair : conformances) {
const auto &protos = pair.second;
auto canonicalProtos = protos;

// canonicalizeProtocols() will sort them and filter out any protocols that
// are refined by other protocols in the list. It should be a no-op at this
// point.
ProtocolType::canonicalizeProtocols(canonicalProtos);

assert(protos.size() == canonicalProtos.size() &&
"redundant conformance requirements");
assert(std::equal(protos.begin(), protos.end(), canonicalProtos.begin()) &&
"out-of-order conformance requirements");
}
}
#endif

Expand Down Expand Up @@ -8382,7 +8405,10 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
auto sig = GenericSignature::get(getGenericParams(), requirements);

#ifndef NDEBUG
if (!Impl->HadAnyError) {
bool hadAnyError = Impl->HadAnyError;

if (requirementSignatureSelfProto &&
!hadAnyError) {
checkGenericSignature(sig.getCanonicalSignature(), *this);
}
#endif
Expand All @@ -8405,6 +8431,13 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
// anything more.
Impl.reset();

#ifndef NDEBUG
if (!requirementSignatureSelfProto &&
!hadAnyError) {
sig.verify();
}
#endif

return sig;
}

Expand Down