[AST] Ensure requirements are correctly identified as conditional.

Previously, the following code would result in different sets of
conditional requirements:

  struct RedundancyOrderDependenceGood<T: P1, U> {}
  extension RedundancyOrderDependenceGood: P2 where U: P1, T == U {}

  struct RedundancyOrderDependenceBad<T, U: P1> {}
  extension RedundancyOrderDependenceBad: P2 where T: P1, T == U {}

The T: P1 requirement is redundant: it is implied from U: P1 and T == U,
but just checking it in the signature of the struct isn't sufficient,
the T == U requirement needs to be considered too. This uses a quadratic
algorithm to identify those cases. We don't think the quadratic-ness is
too bad: most cases have relatively few requirements.

Fixes rdar://problem/34944318.
This commit is contained in:
Huon Wilson
2018-03-09 12:56:25 +11:00
parent 2bb3612f67
commit 88b9f2c94d
4 changed files with 140 additions and 33 deletions

View File

@@ -23,10 +23,17 @@
#include "swift/AST/PrettyStackTrace.h"
#include "swift/AST/Types.h"
#include "swift/Basic/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include <functional>
using namespace swift;
#define DEBUG_TYPE "Generic signature"
STATISTIC(NumRedundantRequirements,
"# of redundant requirements found in signature differencing");
STATISTIC(NumNonRedundantRequirements,
"# of non-redundant requirements found in signature differencing");
void ConformanceAccessPath::print(raw_ostream &out) const {
interleave(begin(), end(),
[&](const Entry &entry) {
@@ -770,6 +777,7 @@ bool GenericSignature::isRequirementSatisfied(Requirement requirement) {
SmallVector<Requirement, 4> GenericSignature::requirementsNotSatisfiedBy(
GenericSignature *otherSig) {
auto &ctxt = getASTContext();
SmallVector<Requirement, 4> result;
// If the signatures are the same, all requirements are satisfied.
@@ -783,10 +791,52 @@ SmallVector<Requirement, 4> GenericSignature::requirementsNotSatisfiedBy(
}
// Find the requirements that aren't satisfied.
for (const auto &req : getRequirements()) {
if (!otherSig->isRequirementSatisfied(req))
result.push_back(req);
}
//
// This is unfortunately quadratic in the size of getRequirements(), but some
// arrangements of signatures result in different canonicalizations that mean
// a requirement in `this` is satisfied by `otherSig`, but not in
// isolation. Specifically, consider:
//
// otherSig == <T, U where U: P1>
// this == <T, U where T: P1, T == U>`
//
// `T: P1` is implied by `U: P1` and `T == U` together, but just checking T:
// P1 in `otherSig` won't find this, because it misses the T == U
// connection. We don't currently know of a great way to handle this in
// general, and instead have to do a brute-force search.
SmallVector<Requirement, 4> redundant;
GenericSignatureBuilder::dropAndCompareEachRequirement(
ctxt, otherSig, this,
/*includeRedundantRequirements=*/false, redundant, result);
NumNonRedundantRequirements += result.size();
NumRedundantRequirements += redundant.size();
#ifndef NDEBUG
// `otherSig + result` should be `this`, so let's check it.
GenericSignatureBuilder builder(ctxt);
// otherSig may have fewer generic parameters, so we can't use
// addGenericSignature directly.
auto source =
GenericSignatureBuilder::FloatingRequirementSource::forAbstract();
for (auto gp : getGenericParams())
builder.addGenericParameter(gp);
for (auto req : otherSig->getRequirements())
builder.addRequirement(req, source, nullptr);
for (auto req : result)
builder.addRequirement(req, source, nullptr);
auto newSig = std::move(builder).computeGenericSignature(
SourceLoc(),
/*allowConcreteGenericParams=*/true,
/*allowBuilderToMove=*/true);
assert(newSig->getCanonicalSignature() == getCanonicalSignature() &&
"signature differencing removed too many requirements");
#endif
return result;
}