Skip to content

[ParseableInterfaces] Skip value witnesses of resilient conformances #20419

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 3 commits into from
Nov 9, 2018
Merged

[ParseableInterfaces] Skip value witnesses of resilient conformances #20419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f116799
[Sema] Factor out ConformanceChecker::resolveValueWitnesses
jrose-apple Nov 7, 2018
a16644c
[Sema] Factor out common code from resolveSingleWitness/ValueWitnesses
jrose-apple Nov 7, 2018
61dd307
[ParseableInterfaces] Skip value witnesses of resilient conformances
jrose-apple Nov 8, 2018
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
268 changes: 138 additions & 130 deletions lib/Sema/TypeCheckProtocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,23 +1173,6 @@ bool WitnessChecker::findBestWitness(
}

if (numViable == 0) {
// Assume any missing value witnesses for a conformance in a parseable
// interface can be treated as opaque.
// FIXME: ...but we should do something better about types.
if (conformance && !conformance->isInvalid()) {
if (auto *SF = DC->getParentSourceFile()) {
if (SF->Kind == SourceFileKind::Interface) {
auto match = matchWitness(TC, ReqEnvironmentCache, Proto,
conformance, DC, requirement, requirement);
assert(match.isViable());
numViable = 1;
bestIdx = matches.size();
matches.push_back(std::move(match));
return true;
}
}
}

if (anyFromUnconstrainedExtension &&
conformance != nullptr &&
conformance->isInvalid()) {
Expand Down Expand Up @@ -2914,6 +2897,23 @@ void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement,
}
}

ResolveWitnessResult
ConformanceChecker::resolveWitnessAsOpaque(ValueDecl *requirement) {
assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
Copy link
Contributor

@harlanhaskins harlanhaskins Nov 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason this doesn't just take an AssociatedTypeDecl? Answered by looking below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole conformance-checking logic is split between type witnesses and non-type witnesses, and in practice it makes sense because they're resolved pretty differently. In this case, you can't actually make an associated type opaque, so the assertion makes sense.

auto match = matchWitness(TC, ReqEnvironmentCache, Proto,
Conformance, DC, requirement, requirement);
recordWitness(requirement, match);
return ResolveWitnessResult::Success;
}

static bool isConformanceFromParseableInterface(
const NormalProtocolConformance *conformance) {
auto *containingSF = conformance->getDeclContext()->getParentSourceFile();
if (!containingSF)
return false;
return containingSF->Kind == SourceFileKind::Interface;
}

ResolveWitnessResult
ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
Expand All @@ -2931,25 +2931,33 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
}
}

// Determine whether we can derive a witness for this requirement.
bool canDerive = false;

// Can a witness for this requirement be derived for this nominal type?
if (auto derivable = DerivedConformance::getDerivableRequirement(
TC,
nominal,
requirement)) {
if (derivable == requirement) {
// If it's the same requirement, we can derive it here.
canDerive = true;
} else {
// Otherwise, go satisfy the derivable requirement, which can introduce
// a member that could in turn satisfy *this* requirement.
auto derivableProto = cast<ProtocolDecl>(derivable->getDeclContext());
if (auto conformance =
TC.conformsToProtocol(Adoptee, derivableProto, DC, None)) {
if (conformance->isConcrete())
(void)conformance->getConcrete()->getWitnessDecl(derivable, &TC);
// Determine whether we can get a witness for this requirement some other way.
bool hasFallbacks = false;
if (!hasFallbacks)
hasFallbacks = requirement->getAttrs().hasAttribute<OptionalAttr>();
if (!hasFallbacks)
hasFallbacks = requirement->getAttrs().isUnavailable(TC.Context);
if (!hasFallbacks)
hasFallbacks = isConformanceFromParseableInterface(Conformance);

if (!hasFallbacks) {
// Can a witness for this requirement be derived for this nominal type?
if (auto derivable = DerivedConformance::getDerivableRequirement(
TC,
nominal,
requirement)) {
if (derivable == requirement) {
// If it's the same requirement, we can derive it here.
hasFallbacks = true;
} else {
// Otherwise, go satisfy the derivable requirement, which can introduce
// a member that could in turn satisfy *this* requirement.
auto derivableProto = cast<ProtocolDecl>(derivable->getDeclContext());
if (auto conformance =
TC.conformsToProtocol(Adoptee, derivableProto, DC, None)) {
if (conformance->isConcrete())
(void)conformance->getConcrete()->getWitnessDecl(derivable, &TC);
}
}
}
}
Expand All @@ -2960,11 +2968,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
unsigned bestIdx = 0;
bool doNotDiagnoseMatches = false;
bool ignoringNames = false;
bool considerRenames =
!canDerive && !requirement->getAttrs().hasAttribute<OptionalAttr>() &&
!requirement->getAttrs().isUnavailable(TC.Context);
if (findBestWitness(requirement,
considerRenames ? &ignoringNames : nullptr,
if (findBestWitness(requirement, hasFallbacks ? nullptr : &ignoringNames,
Conformance,
/* out parameters: */
matches, numViable, bestIdx, doNotDiagnoseMatches)) {
Expand Down Expand Up @@ -3182,19 +3186,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
// We have either no matches or an ambiguous match.

// If we can derive a definition for this requirement, just call it missing.
if (canDerive) {
return ResolveWitnessResult::Missing;
}

// If the requirement is optional, it's okay. We'll satisfy this via
// our handling of default definitions.
//
// FIXME: revisit this once we get default definitions in protocol bodies.
//
// Treat 'unavailable' implicitly as if it were 'optional'.
// The compiler will reject actual uses.
auto Attrs = requirement->getAttrs();
if (Attrs.hasAttribute<OptionalAttr>() || Attrs.isUnavailable(TC.Context)) {
if (hasFallbacks) {
return ResolveWitnessResult::Missing;
}

Expand Down Expand Up @@ -3355,6 +3347,47 @@ CheckTypeWitnessResult swift::checkTypeWitness(TypeChecker &tc, DeclContext *dc,
return CheckTypeWitnessResult();
}

ResolveWitnessResult
ConformanceChecker::resolveWitnessTryingAllStrategies(ValueDecl *requirement) {
using ResolveWitnessStrategy =
decltype(&ConformanceChecker::resolveWitnessViaLookup);
static const constexpr ResolveWitnessStrategy defaultStrategies[] = {
&ConformanceChecker::resolveWitnessViaLookup,
&ConformanceChecker::resolveWitnessViaDerivation,
&ConformanceChecker::resolveWitnessViaDefault};
ArrayRef<ResolveWitnessStrategy> strategies = defaultStrategies;

// Don't try any sort of derivation when processing a parseable interface.
if (isConformanceFromParseableInterface(Conformance)) {
if (Conformance->isResilient()) {
// Resilient conformances don't allow any sort of devirtualization, so
// don't bother looking up witnesses at all.
static const constexpr ResolveWitnessStrategy resilientStrategies[] = {
&ConformanceChecker::resolveWitnessAsOpaque};
strategies = resilientStrategies;
} else {
static const constexpr ResolveWitnessStrategy interfaceStrategies[] = {
&ConformanceChecker::resolveWitnessViaLookup,
&ConformanceChecker::resolveWitnessAsOpaque};
strategies = interfaceStrategies;
}
}

for (auto strategy : strategies) {
ResolveWitnessResult result = (this->*strategy)(requirement);
switch (result) {
case ResolveWitnessResult::Success:
case ResolveWitnessResult::ExplicitFailed:
return result;
case ResolveWitnessResult::Missing:
// Continue trying.
break;
}
}

return ResolveWitnessResult::Missing;
}

/// Attempt to resolve a type witness via member name lookup.
ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
AssociatedTypeDecl *assocType) {
Expand Down Expand Up @@ -3625,60 +3658,14 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
}

#pragma mark Protocol conformance checking
void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
assert(!Conformance->isComplete() && "Conformance is already complete");

FrontendStatsTracer statsTracer(TC.Context.Stats, "check-conformance",
Conformance);

llvm::SaveAndRestore<bool> restoreSuppressDiagnostics(SuppressDiagnostics);
SuppressDiagnostics = false;

// FIXME: Caller checks that this type conforms to all of the
// inherited protocols.

// Emit known diags for this conformance.
emitDelayedDiags();

// If delayed diags have already complained, return.
if (AlreadyComplained) {
Conformance->setInvalid();
return;
}

// Resolve all of the type witnesses.
resolveTypeWitnesses();

// Diagnose missing type witnesses for now.
diagnoseMissingWitnesses(Kind);

// Ensure the conforming type is used.
//
// FIXME: This feels like the wrong place for this, but if we don't put
// it here, extensions don't end up depending on the extended type.
recordConformanceDependency(DC, Adoptee->getAnyNominal(), Conformance, false);

// If we complain about any associated types, there is no point in continuing.
// FIXME: Not really true. We could check witnesses that don't involve the
// failed associated types.
if (AlreadyComplained) {
Conformance->setInvalid();
return;
}

// Diagnose missing value witnesses later.
SWIFT_DEFER { diagnoseMissingWitnesses(Kind); };

// Ensure the associated type conformances are used.
addUsedConformances(Conformance);

// Check non-type requirements.
void ConformanceChecker::resolveValueWitnesses() {
for (auto member : Proto->getMembers()) {
auto requirement = dyn_cast<ValueDecl>(member);
if (!requirement)
continue;

// Associated type requirements handled above.
// Associated type requirements handled elsewhere.
if (isa<TypeDecl>(requirement))
continue;

Expand Down Expand Up @@ -3829,9 +3816,9 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
// If this is an accessor for a storage decl, ignore it.
if (isa<AccessorDecl>(requirement))
continue;
// Try to resolve the witness via explicit definitions.
switch (resolveWitnessViaLookup(requirement)) {

// Try to resolve the witness.
switch (resolveWitnessTryingAllStrategies(requirement)) {
case ResolveWitnessResult::Success:
finalizeWitness();
continue;
Expand All @@ -3841,41 +3828,62 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
continue;

case ResolveWitnessResult::Missing:
// Continue trying below.
// Let it get diagnosed later.
break;
}
}
}

// Try to resolve the witness via derivation.
switch (resolveWitnessViaDerivation(requirement)) {
case ResolveWitnessResult::Success:
finalizeWitness();
continue;
void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
assert(!Conformance->isComplete() && "Conformance is already complete");

case ResolveWitnessResult::ExplicitFailed:
Conformance->setInvalid();
continue;
FrontendStatsTracer statsTracer(TC.Context.Stats, "check-conformance",
Conformance);

case ResolveWitnessResult::Missing:
// Continue trying below.
break;
}
llvm::SaveAndRestore<bool> restoreSuppressDiagnostics(SuppressDiagnostics);
SuppressDiagnostics = false;

// Try to resolve the witness via defaults.
switch (resolveWitnessViaDefault(requirement)) {
case ResolveWitnessResult::Success:
finalizeWitness();
continue;
// FIXME: Caller checks that this type conforms to all of the
// inherited protocols.

case ResolveWitnessResult::ExplicitFailed:
Conformance->setInvalid();
continue;
// Emit known diags for this conformance.
emitDelayedDiags();

case ResolveWitnessResult::Missing:
// Continue trying below.
break;
}
// If delayed diags have already complained, return.
if (AlreadyComplained) {
Conformance->setInvalid();
return;
}

// Resolve all of the type witnesses.
resolveTypeWitnesses();

// Diagnose missing type witnesses for now.
diagnoseMissingWitnesses(Kind);

// Ensure the conforming type is used.
//
// FIXME: This feels like the wrong place for this, but if we don't put
// it here, extensions don't end up depending on the extended type.
recordConformanceDependency(DC, Adoptee->getAnyNominal(), Conformance, false);

// If we complain about any associated types, there is no point in continuing.
// FIXME: Not really true. We could check witnesses that don't involve the
// failed associated types.
if (AlreadyComplained) {
Conformance->setInvalid();
return;
}

// Diagnose missing value witnesses later.
SWIFT_DEFER { diagnoseMissingWitnesses(Kind); };

// Ensure the associated type conformances are used.
addUsedConformances(Conformance);

// Check non-type requirements.
resolveValueWitnesses();

emitDelayedDiags();

// Except in specific hardcoded cases for Foundation/Swift
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/TypeCheckProtocol.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,9 @@ class ConformanceChecker : public WitnessChecker {
void checkNonFinalClassWitness(ValueDecl *requirement,
ValueDecl *witness);

/// Resolve the (non-type) witness as requiring dynamic dispatch.
ResolveWitnessResult resolveWitnessAsOpaque(ValueDecl *requirement);

/// Resolve a (non-type) witness via name lookup.
ResolveWitnessResult resolveWitnessViaLookup(ValueDecl *requirement);

Expand All @@ -629,6 +632,11 @@ class ConformanceChecker : public WitnessChecker {
/// Resolve a (non-type) witness via default definition or optional.
ResolveWitnessResult resolveWitnessViaDefault(ValueDecl *requirement);

/// Resolve a (non-type) witness by trying each standard strategy until one
/// of them produces a result.
ResolveWitnessResult
resolveWitnessTryingAllStrategies(ValueDecl *requirement);

/// Attempt to resolve a type witness via member name lookup.
ResolveWitnessResult resolveTypeWitnessViaLookup(
AssociatedTypeDecl *assocType);
Expand Down Expand Up @@ -675,6 +683,9 @@ class ConformanceChecker : public WitnessChecker {
/// Resolve all of the type witnesses.
void resolveTypeWitnesses();

/// Resolve all of the non-type witnesses.
void resolveValueWitnesses();

/// Resolve the witness for the given non-type requirement as
/// directly as possible, only resolving other witnesses if
/// needed, e.g., to determine type witnesses used within the
Expand Down
Loading