Improve recursion-breaking when resolving type witnesses.

Specifically track when we're resolving type witnesses for a given
conformance, and refuse to recursively attempt to resolve those type
witnesses. The standard library patch in rdar://problem/21789238
triggers this, and isolating it in a small test case has proven
illusive.

Swift SVN r30160
This commit is contained in:
Doug Gregor
2015-07-13 20:06:57 +00:00
parent 0e35bad9dc
commit 161f309975
7 changed files with 134 additions and 84 deletions

View File

@@ -69,17 +69,16 @@ enum class ProtocolConformanceKind {
};
/// Describes the state of a protocol conformance, which may be complete,
/// incomplete, or invalid.
/// incomplete, or currently being checked.
enum class ProtocolConformanceState {
/// The conformance has been fully checked and is complete and well-formed.
/// The conformance has been fully checked.
Complete,
/// The conformance is known but is not yet complete.
Incomplete,
/// The conformance's type witnesses are currently being resolved.
CheckingTypeWitnesses,
/// The conformance is being checked.
Checking,
/// The conformance has been found to be invalid and should not be
/// used.
Invalid
};
/// \brief Describes how a particular type conforms to a given protocol,
@@ -119,19 +118,18 @@ public:
/// Retrieve the state of this conformance.
ProtocolConformanceState getState() const;
/// Determine whether this conformance is complete and well-formed.
/// Determine whether this conformance is complete.
bool isComplete() const {
return getState() == ProtocolConformanceState::Complete;
}
/// Determine whether this conformance is invalid.
bool isInvalid() const {
return getState() == ProtocolConformanceState::Invalid;
}
bool isInvalid() const;
/// Determine whether this conformance is incomplete.
bool isIncomplete() const {
return getState() == ProtocolConformanceState::Incomplete ||
getState() == ProtocolConformanceState::CheckingTypeWitnesses ||
getState() == ProtocolConformanceState::Checking;
}
@@ -295,7 +293,9 @@ class NormalProtocolConformance : public ProtocolConformance,
/// The declaration context containing the ExtensionDecl or
/// NominalTypeDecl that declared the conformance.
DeclContext *DC;
///
/// Also stores the "invalid" bit.
llvm::PointerIntPair<DeclContext *, 1, bool> DCAndInvalid;
/// \brief The mapping of individual requirements in the protocol over to
/// the declarations that satisfy those requirements.
@@ -319,7 +319,7 @@ class NormalProtocolConformance : public ProtocolConformance,
SourceLoc loc, DeclContext *dc,
ProtocolConformanceState state)
: ProtocolConformance(ProtocolConformanceKind::Normal, conformingType),
ProtocolAndState(protocol, state), Loc(loc), DC(dc)
ProtocolAndState(protocol, state), Loc(loc), DCAndInvalid(dc, false)
{
}
@@ -332,7 +332,7 @@ public:
/// Get the declaration context that contains the conforming extension or
/// nominal type declaration.
DeclContext *getDeclContext() const { return DC; }
DeclContext *getDeclContext() const { return DCAndInvalid.getPointer(); }
/// Retrieve the state of this conformance.
ProtocolConformanceState getState() const {
@@ -344,6 +344,12 @@ public:
ProtocolAndState.setInt(state);
}
/// Determine whether this conformance is invalid.
bool isInvalid() const { return DCAndInvalid.getInt(); }
/// Mark this conformance as invalid.
void setInvalid() { DCAndInvalid.setInt(true); }
/// Retrieve the type witness substitution and type decl (if one exists)
/// for the given associated type.
std::pair<const Substitution &, TypeDecl *>
@@ -628,6 +634,10 @@ public:
}
};
inline bool ProtocolConformance::isInvalid() const {
return getRootNormalConformance()->isInvalid();
}
} // end namespace swift
#endif // LLVM_SWIFT_AST_PROTOCOLCONFORMANCE_H

View File

@@ -731,6 +731,11 @@ ArrayRef<Substitution> BoundGenericType::getSubstitutions(
++index;
}
// Before recording substitutions, make sure we didn't end up doing it
// recursively.
if (auto known = ctx.getSubstitutions(canon, gpContext))
return *known;
// Copy and record the substitutions.
auto permanentSubs = ctx.AllocateCopy(resultSubstitutions,
hasTypeVariables

View File

@@ -233,7 +233,8 @@ ConcreteDeclRef NormalProtocolConformance::getWitness(
if (known != Mapping.end()) {
return known->second;
} else {
assert(!isComplete() && "Resolver did not resolve requirement");
assert((!isComplete() || isInvalid()) &&
"Resolver did not resolve requirement");
return ConcreteDeclRef();
}
}

View File

@@ -2497,11 +2497,13 @@ static Type getMemberForBaseType(Module *module,
return Type();
case ConformanceKind::Conforms:
// If we're supposed to skip this conformance's unsatisfied type
// If we have an unsatisfied type witness while we're checking the
// conformances we're supposed to skip this conformance's unsatisfied type
// witnesses, and we have an unsatisfied type witness, return
// "missing".
if (conformance.getPointer()->getRootNormalConformance()
== options.getSkippedConformance() &&
if (conformance.getPointer() &&
conformance.getPointer()->getRootNormalConformance()->getState()
== ProtocolConformanceState::CheckingTypeWitnesses &&
!conformance.getPointer()->hasTypeWitness(assocType, nullptr))
return Type();

View File

@@ -1661,7 +1661,6 @@ struct ASTNodeBase {};
switch (conformance->getState()) {
case ProtocolConformanceState::Complete:
case ProtocolConformanceState::Invalid:
// More checking below.
break;
@@ -1669,6 +1668,7 @@ struct ASTNodeBase {};
// Ignore incomplete conformances; we didn't need them.
return;
case ProtocolConformanceState::CheckingTypeWitnesses:
case ProtocolConformanceState::Checking:
dumpRef(decl);
Out << " has a protocol conformance that is still being checked "

View File

@@ -2581,6 +2581,10 @@ ConformanceChecker::inferTypeWitnessesViaValueWitnesses(ValueDecl *req) {
std::remove_if(witnessResult.Inferred.begin(),
witnessResult.Inferred.end(),
[&](const std::pair<AssociatedTypeDecl *, Type> &result) {
// Filter out errors.
if (result.second->is<ErrorType>())
return true;
// Filter out duplicates.
if (!known.insert({result.first,
result.second->getCanonicalType()})
@@ -2712,8 +2716,7 @@ static Type getWitnessTypeForMatching(TypeChecker &tc,
}
Module *module = conformance->getDeclContext()->getParentModule();
return type.subst(module, substitutions,
SubstOptions(SubstOptions::IgnoreMissing, conformance));
return type.subst(module, substitutions, SubstOptions::IgnoreMissing);
}
/// Remove the 'self' type from the given type, if it's a method type.
@@ -2888,6 +2891,13 @@ namespace {
void ConformanceChecker::resolveTypeWitnesses() {
llvm::SetVector<AssociatedTypeDecl *> unresolvedAssocTypes;
// Track when we are checking type witnesses.
ProtocolConformanceState initialState = Conformance->getState();
Conformance->setState(ProtocolConformanceState::CheckingTypeWitnesses);
defer([&] {
Conformance->setState(initialState);
});
for (auto member : Proto->getMembers()) {
auto assocType = dyn_cast<AssociatedTypeDecl>(member);
if (!assocType)
@@ -3036,28 +3046,51 @@ void ConformanceChecker::resolveTypeWitnesses() {
// Check for completeness of the solution
for (auto assocType : unresolvedAssocTypes) {
// Local function to record a missing associated type.
auto recordMissing = [&] {
if (!missingTypeWitness)
missingTypeWitness = assocType;
};
auto typeWitness = typeWitnesses.begin(assocType);
if (typeWitness == typeWitnesses.end()) {
if (typeWitness != typeWitnesses.end()) {
// The solution contains an error.
if (typeWitness->first->is<ErrorType>()) {
recordMissing();
return;
}
continue;
}
// We don't have a type witness for this associated type.
// If we can form a default type, do so.
if (Type defaultType = computeDefaultTypeWitness(assocType)) {
if (defaultType->is<ErrorType>()) {
recordMissing();
return;
}
typeWitnesses.insert(assocType, {defaultType, reqDepth});
continue;
}
// If we can derive a type witness, do so.
if (Type derivedType = computeDerivedTypeWitness(assocType)) {
if (derivedType->is<ErrorType>()) {
recordMissing();
return;
}
typeWitnesses.insert(assocType, {derivedType, reqDepth});
continue;
}
// The solution is incomplete.
if (!missingTypeWitness)
missingTypeWitness = assocType;
recordMissing();
return;
}
}
// Determine whether there is already a solution with the same
// bindings.
@@ -3212,6 +3245,9 @@ void ConformanceChecker::resolveTypeWitnesses() {
return;
}
// Error cases follow.
Conformance->setInvalid();
// We're going to produce an error below. Mark each unresolved
// associated type witness as erroneous.
for (auto assocType : unresolvedAssocTypes) {
@@ -3422,7 +3458,7 @@ void ConformanceChecker::resolveSingleWitness(ValueDecl *requirement) {
if (requirement->isInvalid()) {
// FIXME: Note that there is no witness?
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
}
@@ -3439,7 +3475,7 @@ void ConformanceChecker::resolveSingleWitness(ValueDecl *requirement) {
for (auto assocType : getReferencedAssociatedTypes(requirement)) {
if (Conformance->getTypeWitness(assocType, nullptr).getReplacement()
->is<ErrorType>()) {
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
}
}
@@ -3450,7 +3486,7 @@ void ConformanceChecker::resolveSingleWitness(ValueDecl *requirement) {
return;
case ResolveWitnessResult::ExplicitFailed:
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
case ResolveWitnessResult::Missing:
@@ -3464,7 +3500,7 @@ void ConformanceChecker::resolveSingleWitness(ValueDecl *requirement) {
return;
case ResolveWitnessResult::ExplicitFailed:
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
case ResolveWitnessResult::Missing:
@@ -3478,7 +3514,7 @@ void ConformanceChecker::resolveSingleWitness(ValueDecl *requirement) {
return;
case ResolveWitnessResult::ExplicitFailed:
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
case ResolveWitnessResult::Missing:
@@ -3506,7 +3542,7 @@ void ConformanceChecker::checkConformance() {
// FIXME: Not really true. We could check witnesses that don't involve the
// failed associated types.
if (AlreadyComplained) {
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
}
@@ -3520,12 +3556,11 @@ void ConformanceChecker::checkConformance() {
TC.checkGenericArguments(DC, Loc, noteLoc, Proto->getDeclaredType(),
Proto->getGenericSignature(),
{Adoptee})) {
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
return;
}
// Check non-type requirements.
bool invalid = false;
for (auto member : Proto->getMembers()) {
auto requirement = dyn_cast<ValueDecl>(member);
if (!requirement)
@@ -3551,7 +3586,7 @@ void ConformanceChecker::checkConformance() {
TC.validateDecl(requirement, true);
if (requirement->isInvalid()) {
invalid = true;
Conformance->setInvalid();
continue;
}
@@ -3566,7 +3601,7 @@ void ConformanceChecker::checkConformance() {
continue;
case ResolveWitnessResult::ExplicitFailed:
invalid = true;
Conformance->setInvalid();
continue;
case ResolveWitnessResult::Missing:
@@ -3580,7 +3615,7 @@ void ConformanceChecker::checkConformance() {
continue;
case ResolveWitnessResult::ExplicitFailed:
invalid = true;
Conformance->setInvalid();
continue;
case ResolveWitnessResult::Missing:
@@ -3594,7 +3629,7 @@ void ConformanceChecker::checkConformance() {
continue;
case ResolveWitnessResult::ExplicitFailed:
invalid = true;
Conformance->setInvalid();
continue;
case ResolveWitnessResult::Missing:
@@ -3604,12 +3639,6 @@ void ConformanceChecker::checkConformance() {
}
emitDelayedDiags();
if (AlreadyComplained || invalid) {
Conformance->setState(ProtocolConformanceState::Invalid);
} else {
Conformance->setState(ProtocolConformanceState::Complete);
}
}
static void diagnoseConformanceFailure(TypeChecker &TC, Type T,
@@ -3643,7 +3672,7 @@ void ConformanceChecker::diagnoseOrDefer(
ValueDecl *requirement, bool isError,
std::function<void(TypeChecker &, NormalProtocolConformance *)> fn) {
if (isError)
Conformance->setState(ProtocolConformanceState::Invalid);
Conformance->setInvalid();
if (SuppressDiagnostics) {
// Stash this in the ASTContext for later emission.
@@ -3689,15 +3718,17 @@ checkConformsToProtocol(TypeChecker &TC,
// Check the conformance below.
break;
case ProtocolConformanceState::CheckingTypeWitnesses:
case ProtocolConformanceState::Checking:
case ProtocolConformanceState::Complete:
// Nothing to do.
return conformance;
case ProtocolConformanceState::Invalid:
case ProtocolConformanceState::Complete:
if (conformance->isInvalid()) {
// Emit any delayed diagnostics and return.
// FIXME: Should we complete checking to emit more diagnostics?
ConformanceChecker(TC, conformance, false).emitDelayedDiags();
}
return conformance;
}
@@ -3710,12 +3741,13 @@ checkConformsToProtocol(TypeChecker &TC,
// Note that we are checking this conformance now.
conformance->setState(ProtocolConformanceState::Checking);
defer([&] { conformance->setState(ProtocolConformanceState::Complete); });
// If the protocol requires a class, non-classes are a non-starter.
if (Proto->requiresClass() && !canT->getClassOrBoundGenericClass()) {
TC.diagnose(ComplainLoc, diag::non_class_cannot_conform_to_class_protocol,
T, Proto->getDeclaredType());
conformance->setState(ProtocolConformanceState::Invalid);
conformance->setInvalid();
return conformance;
}
@@ -3727,7 +3759,7 @@ checkConformsToProtocol(TypeChecker &TC,
TC.diagnose(ComplainLoc,
diag::foreign_class_cannot_conform_to_objc_protocol,
T, Proto->getDeclaredType());
conformance->setState(ProtocolConformanceState::Invalid);
conformance->setInvalid();
return conformance;
}
}
@@ -3737,7 +3769,7 @@ checkConformsToProtocol(TypeChecker &TC,
if (Proto->hasMissingRequirements()) {
TC.diagnose(ComplainLoc, diag::protocol_has_missing_requirements,
T, Proto->getDeclaredType());
conformance->setState(ProtocolConformanceState::Invalid);
conformance->setInvalid();
return conformance;
}
@@ -3759,7 +3791,7 @@ checkConformsToProtocol(TypeChecker &TC,
InheritedProto->getDeclaredType());
}
conformance->setState(ProtocolConformanceState::Invalid);
conformance->setInvalid();
return conformance;
}
}

View File

@@ -267,35 +267,35 @@ func testBrokenConformances1() {
StructWithBrokenConformance.#^BROKEN_CONFORMANCE_1^#
}
// BROKEN_CONFORMANCE_1: Begin completions, 52 items
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DefaultedTypeCommonA{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DefaultedTypeCommonB{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDefaultedTypeB{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DeducedTypeCommonA{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DeducedTypeCommonB{{; name=.+$}}
BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DefaultedTypeCommonA[#DefaultedTypeCommonA#]; name=DefaultedTypeCommonA
BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DefaultedTypeCommonB[#DefaultedTypeCommonB#]; name=DefaultedTypeCommonB
BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDefaultedTypeB[#FooBaseDefaultedTypeB#]; name=FooBaseDefaultedTypeB
BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DeducedTypeCommonA[#DeducedTypeCommonA#]; name=DeducedTypeCommonA
BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DeducedTypeCommonB[#DeducedTypeCommonB#]; name=DeducedTypeCommonB
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonA({#self: Self#})[#() -> Self.DeducedTypeCommonA#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonB({#self: Self#})[#() -> Self.DeducedTypeCommonB#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooBaseB({#self: Self#})[#() -> Int#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooDefaultedType{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooDeducedTypeB{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooDeducedTypeC{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooDeducedTypeD{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooDefaultedType[#FooDefaultedType#]; name=FooDefaultedType
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooDeducedTypeB[#FooDeducedTypeB#]; name=FooDeducedTypeB
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooDeducedTypeC[#FooDeducedTypeC#]; name=FooDeducedTypeC
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooDeducedTypeD[#FooDeducedTypeD#]; name=FooDeducedTypeD
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooB({#self: Self#})[#() -> Self.FooDeducedTypeB#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooC({#self: Self#})[#() -> Self.FooDeducedTypeC#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooD({#self: Self#})[#() -> Self.FooDeducedTypeD#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DefaultedTypeCommonC{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DefaultedTypeCommonD{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDefaultedTypeA{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDefaultedTypeC{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DeducedTypeCommonC{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: DeducedTypeCommonD{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DefaultedTypeCommonC[#DefaultedTypeCommonC#]; name=DefaultedTypeCommonC
// BROKEN_CONFORMANCE_1-DAG:Decl[TypeAlias]/CurrNominal: DefaultedTypeCommonD[#DefaultedTypeCommonD#]; name=DefaultedTypeCommonD
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDefaultedTypeA[#FooBaseDefaultedTypeA#]; name=FooBaseDefaultedTypeA
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDefaultedTypeC[#FooBaseDefaultedTypeC#]; name=FooBaseDefaultedTypeC
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DeducedTypeCommonC[#DeducedTypeCommonC#]; name=DeducedTypeCommonC
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: DeducedTypeCommonD[#DeducedTypeCommonD#]; name=DeducedTypeCommonD
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonA({#self: Self#})[#() -> Self.DeducedTypeCommonA#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonB({#self: Self#})[#() -> Self.DeducedTypeCommonB#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonC({#self: Self#})[#() -> Self.DeducedTypeCommonC#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceCommonD({#self: Self#})[#() -> Self.DeducedTypeCommonD#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDeducedTypeA{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDeducedTypeB{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDeducedTypeC{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/Super: FooBaseDeducedTypeD{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDeducedTypeA[#FooBaseDeducedTypeA#]; name=FooBaseDeducedTypeA
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDeducedTypeB[#FooBaseDeducedTypeB#]; name=FooBaseDeducedTypeB
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDeducedTypeC[#FooBaseDeducedTypeC#]; name=FooBaseDeducedTypeC
// BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal: FooBaseDeducedTypeD[#FooBaseDeducedTypeD#]; name=FooBaseDeducedTypeD
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooBaseA({#self: Self#})[#() -> Self.FooBaseDeducedTypeA#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooBaseB({#self: Self#})[#() -> Self.FooBaseDeducedTypeB#]{{; name=.+$}}
// BROKEN_CONFORMANCE_1-DAG: Decl[InstanceMethod]/Super: deduceFooBaseC({#self: Self#})[#() -> Self.FooBaseDeducedTypeC#]{{; name=.+$}}