Skip to content

GSB: Move signature verification from CanGenericSignature::getCanonical() to computeGenericSignature() #36411

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
Mar 12, 2021
Merged

GSB: Move signature verification from CanGenericSignature::getCanonical() to computeGenericSignature() #36411

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
10 changes: 5 additions & 5 deletions include/swift/AST/GenericSignature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class CanGenericSignature : public GenericSignature {
/// requirements, first canonicalizing the types.
static CanGenericSignature
getCanonical(TypeArrayView<GenericTypeParamType> params,
ArrayRef<Requirement> requirements, bool skipValidation = false);
ArrayRef<Requirement> requirements);

public:
CanGenericSignature(std::nullptr_t) : GenericSignature(nullptr) {}
Expand Down Expand Up @@ -342,6 +342,8 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
///
/// The type parameters must be known to not be concrete within the context.
bool areSameTypeParameterInContext(Type type1, Type type2) const;
bool areSameTypeParameterInContext(Type type1, Type type2,
GenericSignatureBuilder &builder) const;

/// Determine if \c sig can prove \c requirement, meaning that it can deduce
/// T: Foo or T == U (etc.) with the information it knows. This includes
Expand All @@ -360,12 +362,10 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
/// Return the canonical version of the given type under this generic
/// signature.
CanType getCanonicalTypeInContext(Type type) const;
bool isCanonicalTypeInContext(Type type) const;

/// Return the canonical version of the given type under this generic
/// signature.
CanType getCanonicalTypeInContext(Type type,
GenericSignatureBuilder &builder) const;

bool isCanonicalTypeInContext(Type type) const;
bool isCanonicalTypeInContext(Type type,
GenericSignatureBuilder &builder) const;

Expand Down
137 changes: 14 additions & 123 deletions lib/AST/GenericSignature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,23 +176,9 @@ bool GenericSignatureImpl::isCanonical() const {
return getCanonicalSignature().getPointer() == this;
}

#ifndef NDEBUG
/// Determine the canonical ordering of requirements.
static unsigned getRequirementKindOrder(RequirementKind kind) {
switch (kind) {
case RequirementKind::Conformance: return 2;
case RequirementKind::Superclass: return 0;
case RequirementKind::SameType: return 3;
case RequirementKind::Layout: return 1;
}
llvm_unreachable("unhandled kind");
}
#endif

CanGenericSignature
CanGenericSignature::getCanonical(TypeArrayView<GenericTypeParamType> params,
ArrayRef<Requirement> requirements,
bool skipValidation) {
ArrayRef<Requirement> requirements) {
// Canonicalize the parameters and requirements.
SmallVector<GenericTypeParamType*, 8> canonicalParams;
canonicalParams.reserve(params.size());
Expand All @@ -205,116 +191,9 @@ CanGenericSignature::getCanonical(TypeArrayView<GenericTypeParamType> params,
for (auto &reqt : requirements)
canonicalRequirements.push_back(reqt.getCanonical());

(void)skipValidation;
auto canSig = get(canonicalParams, canonicalRequirements,
/*isKnownCanonical=*/true);

#ifndef NDEBUG
if (skipValidation)
return CanGenericSignature(canSig);

PrettyStackTraceGenericSignature debugStack("canonicalizing", canSig);

// 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()) &&
"Left-hand side is not canonical");
assert(canSig->isCanonicalTypeInContext(reqt.getSecondType()) &&
"Superclass type isn't canonical in its own context");
break;

case RequirementKind::Layout:
assert(canSig->isCanonicalTypeInContext(reqt.getFirstType()) &&
"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());
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) &&
"Concrete same-type isn't canonical in its own context");
}
break;
}

case RequirementKind::Conformance:
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");
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())) {
// 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");
}
}

// From here on, we only care about cases where the previous and current
// requirements have the same left-hand side.
if (compareLHS != 0) continue;

// Check ordering of requirement kinds.
assert((getRequirementKindOrder(prevReqt.getKind()) <=
getRequirementKindOrder(reqt.getKind())) &&
"Requirements for a given kind are out-of-order");

// From here on, we only care about the same requirement kind.
if (prevReqt.getKind() != reqt.getKind()) continue;

assert(reqt.getKind() == RequirementKind::Conformance &&
"Only conformance requirements can have multiples");

auto prevProto = prevReqt.getProtocolDecl();
auto proto = reqt.getProtocolDecl();
assert(TypeDecl::compare(prevProto, proto) < 0 &&
"Out-of-order conformance requirements");
}
#endif

return CanGenericSignature(canSig);
}

Expand Down Expand Up @@ -519,7 +398,19 @@ bool GenericSignatureImpl::areSameTypeParameterInContext(Type type1,
if (type1.getPointer() == type2.getPointer())
return true;

auto &builder = *getGenericSignatureBuilder();
return areSameTypeParameterInContext(type1, type2,
*getGenericSignatureBuilder());
}

bool GenericSignatureImpl::areSameTypeParameterInContext(Type type1,
Type type2,
GenericSignatureBuilder &builder) const {
assert(type1->isTypeParameter());
assert(type2->isTypeParameter());

if (type1.getPointer() == type2.getPointer())
return true;

auto equivClass1 =
builder.resolveEquivalenceClass(
type1,
Expand Down
129 changes: 129 additions & 0 deletions lib/AST/GenericSignatureBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7948,6 +7948,129 @@ void GenericSignatureBuilder::addGenericSignature(GenericSignature sig) {
addRequirement(reqt, FloatingRequirementSource::forAbstract(), nullptr);
}

#ifndef NDEBUG

/// Determine the canonical ordering of requirements.
static unsigned getRequirementKindOrder(RequirementKind kind) {
switch (kind) {
case RequirementKind::Conformance: return 2;
case RequirementKind::Superclass: return 0;
case RequirementKind::SameType: return 3;
case RequirementKind::Layout: return 1;
}
llvm_unreachable("unhandled kind");
}

static void checkGenericSignature(CanGenericSignature canSig,
GenericSignatureBuilder &builder) {
PrettyStackTraceGenericSignature debugStack("checking", canSig);

auto canonicalRequirements = canSig->getRequirements();

// 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) &&
"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");
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");
}
}

// From here on, we only care about cases where the previous and current
// requirements have the same left-hand side.
if (compareLHS != 0) continue;

// Check ordering of requirement kinds.
assert((getRequirementKindOrder(prevReqt.getKind()) <=
getRequirementKindOrder(reqt.getKind())) &&
"Requirements for a given kind are out-of-order");

// From here on, we only care about the same requirement kind.
if (prevReqt.getKind() != reqt.getKind()) continue;

assert(reqt.getKind() == RequirementKind::Conformance &&
"Only conformance requirements can have multiples");

auto prevProto = prevReqt.getProtocolDecl();
auto proto = reqt.getProtocolDecl();
assert(TypeDecl::compare(prevProto, proto) < 0 &&
"Out-of-order conformance requirements");
}
}
#endif

GenericSignature GenericSignatureBuilder::computeGenericSignature(
bool allowConcreteGenericParams,
bool allowBuilderToMove) && {
Expand All @@ -7961,6 +8084,12 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
// Form the generic signature.
auto sig = GenericSignature::get(getGenericParams(), requirements);

#ifndef NDEBUG
if (!Impl->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 Down
5 changes: 1 addition & 4 deletions lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2452,13 +2452,10 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
requirementsSig->print(llvm::errs());
llvm::errs() << "\n";

// Note: One cannot canonicalize a requirement signature, because
// requirement signatures are necessarily missing requirements.
llvm::errs() << "Canonical requirement signature: ";
auto canRequirementSig =
CanGenericSignature::getCanonical(requirementsSig->getGenericParams(),
requirementsSig->getRequirements(),
/*skipValidation=*/true);
requirementsSig->getRequirements());
canRequirementSig->print(llvm::errs());
llvm::errs() << "\n";
}
Expand Down
Loading