Skip to content

AST: Use RequirementMachine to verify protocol requirement signatures #40482

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.

Sign up for GitHub

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 1 commit into from
Dec 9, 2021
Merged

AST: Use RequirementMachine to verify protocol requirement signatures #40482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions include/swift/AST/GenericSignature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ class GenericSignature {

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

/// Check invariants for a list of requirements that are understood to
/// be valid in the given signature; used to verify a protocol's
/// requirement signature against the protocol generic signature
/// <Self where Self : P>.
void verify(ArrayRef<Requirement> reqts) const;
};

/// A reference to a canonical generic signature.
Expand Down
14 changes: 7 additions & 7 deletions lib/AST/GenericSignature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,20 +1564,20 @@ int swift::compareDependentTypes(Type type1, Type type2) {
#pragma mark Generic signature verification

void GenericSignature::verify() const {
verify(getRequirements());
}

void GenericSignature::verify(ArrayRef<Requirement> reqts) 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];
for (unsigned idx : indices(reqts)) {
const auto &reqt = reqts[idx].getCanonical();

// Left-hand side must be a canonical type parameter.
if (reqt.getKind() != RequirementKind::SameType) {
Expand Down Expand Up @@ -1661,7 +1661,7 @@ void GenericSignature::verify() const {
if (idx == 0) continue;

// Make sure that the left-hand sides are in nondecreasing order.
const auto &prevReqt = canonicalRequirements[idx-1];
const auto &prevReqt = reqts[idx-1];
int compareLHS =
compareDependentTypes(prevReqt.getFirstType(), reqt.getFirstType());
if (compareLHS > 0) {
Expand Down
137 changes: 2 additions & 135 deletions lib/AST/GenericSignatureBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8025,132 +8025,6 @@ void GenericSignatureBuilder::addGenericSignature(GenericSignature sig) {
addRequirement(reqt, FloatingRequirementSource::forAbstract(), nullptr);
}

#ifndef NDEBUG

static void checkGenericSignature(CanGenericSignature canSig,
GenericSignatureBuilder &builder) {
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 signature is canonical.
for (unsigned idx : indices(canonicalRequirements)) {
debugStack.setRequirement(idx);

const auto &reqt = canonicalRequirements[idx];

// Left-hand side must be canonical in its context.
// Check canonicalization of requirement itself.
switch (reqt.getKind()) {
case RequirementKind::Superclass:
assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) &&
"Left-hand side is not canonical");
assert(canSig->isCanonicalTypeInContext(reqt.getSecondType(), builder) &&
"Superclass type isn't canonical in its own context");
break;

case RequirementKind::Layout:
assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) &&
"Left-hand side is not canonical");
break;

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

auto firstType = reqt.getFirstType();
auto secondType = reqt.getSecondType();
assert(isCanonicalAnchor(firstType));

if (reqt.getSecondType()->isTypeParameter()) {
assert(isCanonicalAnchor(secondType));
assert(compareDependentTypes(firstType, secondType) < 0 &&
"Out-of-order type parameters in same-type constraint");
} else {
assert(canSig->isCanonicalTypeInContext(secondType, builder) &&
"Concrete same-type isn't canonical in its own context");
}
break;
}

case RequirementKind::Conformance:
assert(canSig->isCanonicalTypeInContext(reqt.getFirstType(), builder) &&
"Left-hand side is not canonical");
assert(reqt.getFirstType()->isTypeParameter() &&
"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;
}

// 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());
assert(compareLHS <= 0 && "Out-of-order left-hand sides");

// 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(),
builder)) {
// If it's a it's a type parameter, make sure the equivalence class is
// wired together sanely.
if (prevReqt.getSecondType()->isTypeParameter()) {
assert(prevReqt.getSecondType()->isEqual(reqt.getFirstType()) &&
"same-type constraints within an equiv. class are out-of-order");
} else {
// Otherwise, the concrete types must match up.
assert(prevReqt.getSecondType()->isEqual(reqt.getSecondType()) &&
"inconsistent concrete same-type constraints in equiv. class");
}
}

// 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()) {
assert(compareLHS < 0 &&
"Concrete subject type should not have any other requirements");
}

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

static Type stripBoundDependentMemberTypes(Type t) {
if (auto *depMemTy = t->getAs<DependentMemberType>()) {
return DependentMemberType::get(
Expand Down Expand Up @@ -8444,15 +8318,8 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
// Form the generic signature.
auto sig = GenericSignature::get(getGenericParams(), requirements);

#ifndef NDEBUG
bool hadAnyError = Impl->HadAnyError;

if (requirementSignatureSelfProto &&
!hadAnyError) {
checkGenericSignature(sig.getCanonicalSignature(), *this);
}
#endif

// When we can, move this generic signature builder to make it the canonical
// builder, rather than constructing a new generic signature builder that
// will produce the same thing.
Expand All @@ -8461,7 +8328,7 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
//
// Also, we cannot do this when building a requirement signature.
if (requirementSignatureSelfProto == nullptr &&
!Impl->HadAnyError) {
!hadAnyError) {
// Register this generic signature builder as the canonical builder for the
// given signature.
Context.registerGenericSignatureBuilder(sig, std::move(*this));
Expand All @@ -8472,7 +8339,7 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
Impl.reset();

#ifndef NDEBUG
if (!requirementSignatureSelfProto &&
if (requirementSignatureSelfProto == nullptr &&
!hadAnyError) {
sig.verify();
}
Expand Down
16 changes: 12 additions & 4 deletions lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2618,10 +2618,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
if (!SF || SF->Kind != SourceFileKind::Interface)
TypeChecker::inferDefaultWitnesses(PD);

// Explicity compute the requirement signature to detect errors.
auto reqSig = PD->getRequirementSignature();

if (PD->getASTContext().TypeCheckerOpts.DebugGenericSignatures) {
auto requirementsSig =
GenericSignature::get({PD->getProtocolSelfType()},
PD->getRequirementSignature());
GenericSignature::get({PD->getProtocolSelfType()}, reqSig);

llvm::errs() << "\n";
llvm::errs() << "Protocol requirement signature:\n";
Expand All @@ -2639,8 +2641,14 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
llvm::errs() << "\n";
}

// Explicity compute the requirement signature to detect errors.
(void) PD->getRequirementSignature();

#ifndef NDEBUG
// In asserts builds, also verify some invariants of the requirement
// signature.
PD->getGenericSignature().verify(reqSig);
#endif

(void) reqSig;

checkExplicitAvailability(PD);
}
Expand Down