Skip to content

[CSFix] Use fully qualified locators for requirement failures #22693

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 1 commit into from
Feb 18, 2019
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
4 changes: 3 additions & 1 deletion lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ Expr *FailureDiagnostic::findParentExpr(Expr *subExpr) const {
}

Type RequirementFailure::getOwnerType() const {
return getType(getAnchor())->getInOutObjectType()->getMetatypeInstanceType();
return getType(getRawAnchor())
->getInOutObjectType()
->getMetatypeInstanceType();
}

const GenericContext *RequirementFailure::getGenericContext() const {
Expand Down
23 changes: 21 additions & 2 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class RequirementFailure : public FailureDiagnostic {
if (!expr)
return;

if (auto *parentExpr = findParentExpr(getAnchor()))
if (auto *parentExpr = findParentExpr(getRawAnchor()))
Apply = dyn_cast<ApplyExpr>(parentExpr);
}

Expand Down Expand Up @@ -254,12 +254,31 @@ class RequirementFailure : public FailureDiagnostic {
if (isConditional())
return true;

auto *anchor = getAnchor();
// In the situations like this:
//
// ```swift
// enum E<T: P> { case foo(T) }
// let _: E = .foo(...)
// ```
//
// `E` is going to be opened twice. First, when
// it's used as a contextual type, and when `E.foo`
// is found and its function type is opened.
// We still want to record both fixes but should
// avoid diagnosing the same problem multiple times.
if (isa<UnresolvedMemberExpr>(anchor)) {
auto path = getLocator()->getPath();
if (path.front().getKind() != ConstraintLocator::UnresolvedMember)
return false;
}

// For static/initializer calls there is going to be
// a separate fix, attached to the argument, which is
// much easier to diagnose.
// For operator calls we can't currently produce a good
// diagnostic, so instead let's refer to expression diagnostics.
return !(Apply && (isOperator(Apply) || isa<TypeExpr>(getAnchor())));
return !(Apply && (isOperator(Apply) || isa<TypeExpr>(anchor)));
}

static bool isOperator(const ApplyExpr *apply) {
Expand Down
37 changes: 9 additions & 28 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1612,29 +1612,22 @@ static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1,
if (type1->hasDependentMember() || type2->hasDependentMember())
return nullptr;

auto reqPath = path.take_back(2);
auto req = reqPath.back();

ConstraintLocator *reqLoc = nullptr;
auto req = path.back();
if (req.isConditionalRequirement()) {
// path is - ... -> open generic -> type req # -> cond req #,
// to identify type requirement we only need `open generic -> type req #`
// part, because that's how fixes for type requirements are recorded.
reqPath = path.drop_back().take_back(2);
auto reqPath = path.drop_back();
// If underlying conformance requirement has been fixed,
// then there is no reason to fix up conditional requirements.
if (cs.hasFixFor(cs.getConstraintLocator(anchor, reqPath,
/*summaryFlags=*/0)))
return nullptr;

// For conditional requirements we need a full path.
reqLoc = cs.getConstraintLocator(anchor, path, /*summaryFlags=*/0);
} else {
// Build simplified locator which only contains anchor and requirement info.
reqLoc = cs.getConstraintLocator(anchor, reqPath,
/*summaryFlags=*/0);
}

auto *reqLoc = cs.getConstraintLocator(anchor, path,
/*summaryFlags=*/0);

auto reqKind = static_cast<RequirementKind>(req.getValue2());
switch (reqKind) {
case RequirementKind::SameType: {
Expand Down Expand Up @@ -2789,32 +2782,20 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(

if (path.back().isTypeParameterRequirement() ||
path.back().isConditionalRequirement()) {
ConstraintLocator *reqLoc = nullptr;
if (path.back().isConditionalRequirement()) {
// Drop 'conditional requirement' element remainder
// Drop 'conditional requirement' element, remainder
// of the path is going to point to type requirement
// this conditional comes from.
auto reqPath = ArrayRef<LocatorPathElt>(path).drop_back();
// Underlying conformance requirement is itself fixed,
// this wouldn't lead to a right solution.
if (hasFixFor(getConstraintLocator(anchor, reqPath.take_back(2),
if (hasFixFor(getConstraintLocator(anchor, reqPath,
/*summaryFlags=*/0)))
return SolutionKind::Error;

// For conditional requirements we need complete path, which includes
// type requirement position, to be able to fetch conformance later.
reqLoc = getConstraintLocator(locator);
} else {
// Let's strip all of the unnecessary information from locator,
// diagnostics only care about anchor - to lookup type,
// generic signature where requirement comes from, and
// what was the requirement# which is not satisfied.
auto reqPath = ArrayRef<LocatorPathElt>(path).take_back(2);
reqLoc = getConstraintLocator(anchor, reqPath,
/*summaryFlags=*/0);
}

auto *fix = MissingConformance::create(*this, type, protocol, reqLoc);
auto *fix = MissingConformance::create(*this, type, protocol,
getConstraintLocator(locator));
if (!recordFix(fix))
return SolutionKind::Solved;
}
Expand Down