Skip to content

[Diagnostics] Special case requirement failures related to return statement/expression #35281

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.

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
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,20 @@ class LocatorPathElt::ArgumentAttribute final : public StoredIntegerElement<1> {
}
};

class LocatorPathElt::ConformanceRequirement final
: public StoredPointerElement<ProtocolDecl> {
public:
ConformanceRequirement(ProtocolDecl *protocol)
: StoredPointerElement(PathElementKind::ConformanceRequirement,
protocol) {}

ProtocolDecl *getRequirement() const { return getStoredPointer(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == ConstraintLocator::ConformanceRequirement;
}
};

namespace details {
template <typename CustomPathElement>
class PathElement {
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ CUSTOM_LOCATOR_PATH_ELT(ArgumentAttribute)
/// The result of a chain of member accesses off an UnresolvedMemberExpr
SIMPLE_LOCATOR_PATH_ELT(UnresolvedMemberChainResult)

/// The conformance requirement associated with a type.
CUSTOM_LOCATOR_PATH_ELT(ConformanceRequirement)

#undef LOCATOR_PATH_ELT
#undef CUSTOM_LOCATOR_PATH_ELT
#undef SIMPLE_LOCATOR_PATH_ELT
Expand Down
60 changes: 30 additions & 30 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7971,10 +7971,12 @@ static Optional<SolutionApplicationTarget> applySolutionToForEachStmt(
auto stmt = forEachStmtInfo.stmt;
auto sequenceProto = TypeChecker::getProtocol(
cs.getASTContext(), stmt->getForLoc(), KnownProtocolKind::Sequence);
auto contextualLocator = cs.getConstraintLocator(
target.getAsExpr(), LocatorPathElt::ContextualType());
auto sequenceConformance = solution.resolveConformance(
contextualLocator, sequenceProto);
auto contextualLocator = solution.getConstraintLocator(
target.getAsExpr(),
{LocatorPathElt::ContextualType(),
LocatorPathElt::ConformanceRequirement(sequenceProto)});
auto sequenceConformance =
solution.resolveConformance(contextualLocator, sequenceProto);
assert(!sequenceConformance.isInvalid() &&
"Couldn't find sequence conformance");

Expand Down Expand Up @@ -8385,32 +8387,30 @@ Expr *Solution::coerceToType(Expr *expr, Type toType,

ProtocolConformanceRef Solution::resolveConformance(
ConstraintLocator *locator, ProtocolDecl *proto) {
for (const auto &conformance : Conformances) {
if (conformance.first != locator)
continue;
if (conformance.second.getRequirement() != proto)
continue;

// If the conformance doesn't require substitution, return it immediately.
auto conformanceRef = conformance.second;
if (conformanceRef.isAbstract())
return conformanceRef;

auto concrete = conformanceRef.getConcrete();
auto conformingType = concrete->getType();
if (!conformingType->hasTypeVariable())
return conformanceRef;

// Substitute into the conformance type, then look for a conformance
// again.
// FIXME: Should be able to perform the substitution using the Solution
// itself rather than another conforms-to-protocol check.
Type substConformingType = simplifyType(conformingType);
return TypeChecker::conformsToProtocol(
substConformingType, proto, constraintSystem->DC);
}

return ProtocolConformanceRef::forInvalid();
auto conformance = llvm::find_if(Conformances, [&locator](const auto &elt) {
return elt.first == locator;
});

if (conformance == Conformances.end())
return ProtocolConformanceRef::forInvalid();

// If the conformance doesn't require substitution, return it immediately.
auto conformanceRef = conformance->second;
if (conformanceRef.isAbstract())
return conformanceRef;

auto concrete = conformanceRef.getConcrete();
auto conformingType = concrete->getType();
if (!conformingType->hasTypeVariable())
return conformanceRef;

// Substitute into the conformance type, then look for a conformance
// again.
// FIXME: Should be able to perform the substitution using the Solution
// itself rather than another conforms-to-protocol check.
Type substConformingType = simplifyType(conformingType);
return TypeChecker::conformsToProtocol(substConformingType, proto,
constraintSystem->DC);
}

bool Solution::hasType(ASTNode node) const {
Expand Down
28 changes: 24 additions & 4 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,12 @@ ProtocolConformance *RequirementFailure::getConformanceForConditionalReq(
return nullptr;

auto path = locator->getPath();
auto *typeReqLoc = getConstraintLocator(getRawAnchor(), path.drop_back());
auto *conformanceLoc = getConstraintLocator(getRawAnchor(), path.drop_back());

auto result = llvm::find_if(
solution.Conformances,
[&](const std::pair<ConstraintLocator *, ProtocolConformanceRef>
&conformance) { return conformance.first == typeReqLoc; });
&conformance) { return conformance.first == conformanceLoc; });
assert(result != solution.Conformances.end());

auto conformance = result->second;
Expand Down Expand Up @@ -249,8 +249,28 @@ ValueDecl *RequirementFailure::getDeclRef() const {
return getAffectedDeclFromType(func->getResultInterfaceType());
}

if (isFromContextualType())
return getAffectedDeclFromType(getContextualType(getRawAnchor()));
if (isFromContextualType()) {
auto anchor = getRawAnchor();
auto contextualPurpose = getContextualTypePurpose(anchor);
auto contextualTy = getContextualType(anchor);

// If the issue is a mismatch between `return` statement/expression
// and its contextual requirements, it means that affected declaration
// is a declarer of a contextual "result" type e.g. member of a
// type, local function etc.
if (contextualPurpose == CTP_ReturnStmt ||
contextualPurpose == CTP_ReturnSingleExpr) {
// In case of opaque result type, let's point to the declaration
// associated with the type itself (since it has one) instead of
// declarer.
if (auto *opaque = contextualTy->getAs<OpaqueTypeArchetypeType>())
return opaque->getDecl();

return cast<ValueDecl>(getDC()->getAsDecl());
}

return getAffectedDeclFromType(contextualTy);
}

if (auto overload = getCalleeOverloadChoiceIfAvailable(getLocator())) {
// If there is a declaration associated with this
Expand Down
14 changes: 8 additions & 6 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5710,21 +5710,23 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
/// Record the given conformance as the result, adding any conditional
/// requirements if necessary.
auto recordConformance = [&](ProtocolConformanceRef conformance) {
auto *conformanceLoc = getConstraintLocator(
loc, LocatorPathElt::ConformanceRequirement(protocol));
// Record the conformance.
CheckedConformances.push_back({loc, conformance});
CheckedConformances.push_back({conformanceLoc, conformance});

if (isConformanceUnavailable(conformance, loc))
if (isConformanceUnavailable(conformance, conformanceLoc))
increaseScore(SK_Unavailable);

// This conformance may be conditional, in which case we need to consider
// those requirements as constraints too.
if (conformance.isConcrete()) {
unsigned index = 0;
for (const auto &req : conformance.getConditionalRequirements()) {
addConstraint(req,
locator.withPathElement(
LocatorPathElt::ConditionalRequirement(
index++, req.getKind())));
addConstraint(
req, getConstraintLocator(conformanceLoc,
LocatorPathElt::ConditionalRequirement(
index++, req.getKind())));
}
}

Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
case ConstraintLocator::KeyPathComponent:
case ConstraintLocator::ConditionalRequirement:
case ConstraintLocator::TypeParameterRequirement:
case ConstraintLocator::ConformanceRequirement:
case ConstraintLocator::ImplicitlyUnwrappedDisjunctionChoice:
case ConstraintLocator::DynamicLookupResult:
case ConstraintLocator::ContextualType:
Expand Down Expand Up @@ -397,6 +398,14 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
break;
}

case ConformanceRequirement: {
auto reqElt = elt.castTo<LocatorPathElt::ConformanceRequirement>();
out << "conformance requirement (";
reqElt.getRequirement()->dumpRef(out);
out << ")";
break;
}

case ImplicitlyUnwrappedDisjunctionChoice:
out << "implicitly unwrapped disjunction choice";
break;
Expand Down
6 changes: 3 additions & 3 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5180,15 +5180,15 @@ static Optional<Requirement> getRequirement(ConstraintSystem &cs,

if (reqLoc->isConditionalRequirement()) {
auto path = reqLocator->getPath();
auto *typeReqLoc =
auto *conformanceLoc =
cs.getConstraintLocator(reqLocator->getAnchor(), path.drop_back());

auto conformances = cs.getCheckedConformances();
auto result = llvm::find_if(
conformances,
[&typeReqLoc](
[&conformanceLoc](
const std::pair<ConstraintLocator *, ProtocolConformanceRef>
&conformance) { return conformance.first == typeReqLoc; });
&conformance) { return conformance.first == conformanceLoc; });
assert(result != conformances.end());

auto conformance = result->second;
Expand Down
29 changes: 29 additions & 0 deletions test/Constraints/sr13992.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %target-typecheck-verify-swift

// SR-13992

protocol V {}

protocol P1 {}
protocol P2 {
func bar() -> V
}
protocol P3 {}

struct S<T> {
var foo: T
}

extension S : P1 {}
extension S : P2 where T: P3 { // expected-note {{requirement from conditional conformance of 'S<V>' to 'P2'}}
func bar() -> V { fatalError() }
}

struct Q {
var foo: V

func test() -> P1 & P2 {
S(foo: foo) // expected-error {{protocol 'V' as a type cannot conform to 'P3'}}
// expected-note@-1 {{only concrete types such as structs, enums and classes can conform to protocols}}
}
}