Skip to content

[ConstraintSystem] Finish porting unresolved member reference failures. #29000

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
Jan 5, 2020
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
60 changes: 0 additions & 60 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
bool visitIdentityExpr(IdentityExpr *E);
bool visitTryExpr(TryExpr *E);

bool visitUnresolvedMemberExpr(UnresolvedMemberExpr *E);
bool visitUnresolvedDotExpr(UnresolvedDotExpr *UDE);
bool visitArrayExpr(ArrayExpr *E);
bool visitDictionaryExpr(DictionaryExpr *E);
Expand Down Expand Up @@ -2126,65 +2125,6 @@ bool FailureDiagnosis::visitObjectLiteralExpr(ObjectLiteralExpr *E) {
return true;
}

bool FailureDiagnosis::visitUnresolvedMemberExpr(UnresolvedMemberExpr *E) {
// If we have no contextual type, there is no way to resolve this. Just
// diagnose this as an ambiguity.
if (!CS.getContextualType())
return false;

// OTOH, if we do have a contextual type, we can provide a more specific
// error. Dig out the UnresolvedValueMember constraint for this expr node.
Constraint *memberConstraint = nullptr;
auto checkConstraint = [&](Constraint *C) {
if (C->getKind() == ConstraintKind::UnresolvedValueMember &&
simplifyLocatorToAnchor(C->getLocator()) == E)
memberConstraint = C;
};

if (CS.failedConstraint)
checkConstraint(CS.failedConstraint);
for (auto &C : CS.getConstraints()) {
if (memberConstraint) break;
checkConstraint(&C);
}

// If we can't find the member constraint in question, then we failed.
if (!memberConstraint)
return false;

std::function<bool(ArrayRef<OverloadChoice>)> callback = [&](
ArrayRef<OverloadChoice> candidates) {
bool hasTrailingClosure = callArgHasTrailingClosure(E->getArgument());

// Dump all of our viable candidates into a CalleeCandidateInfo & sort it
// out.
CalleeCandidateInfo candidateInfo(Type(), candidates, hasTrailingClosure,
CS);

// Filter the candidate list based on the argument we may or may not have.
candidateInfo.filterContextualMemberList(E->getArgument());

// If we have multiple candidates, then we have an ambiguity.
if (candidateInfo.size() != 1) {
SourceRange argRange;
if (auto arg = E->getArgument())
argRange = arg->getSourceRange();
diagnose(E->getNameLoc(), diag::ambiguous_member_overload_set,
E->getName())
.highlight(argRange);
candidateInfo.suggestPotentialOverloads(E->getNameLoc().getBaseNameLoc());
return true;
}

return false;
};

return diagnoseMemberFailures(E, nullptr, memberConstraint->getKind(),
memberConstraint->getMember(),
memberConstraint->getFunctionRefKind(),
memberConstraint->getLocator(), callback);
}

bool FailureDiagnosis::diagnoseMemberFailures(
Expr *E, Expr *baseExpr, ConstraintKind lookupKind, DeclNameRef memberName,
FunctionRefKind funcRefKind, ConstraintLocator *locator,
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,14 +472,15 @@ UseSubscriptOperator *UseSubscriptOperator::create(ConstraintSystem &cs,
bool DefineMemberBasedOnUse::diagnose(bool asNote) const {
auto failure = MissingMemberFailure(getConstraintSystem(), BaseType,
Name, getLocator());
return failure.diagnose(asNote);
return AlreadyDiagnosed || failure.diagnose(asNote);
}

DefineMemberBasedOnUse *
DefineMemberBasedOnUse::create(ConstraintSystem &cs, Type baseType,
DeclNameRef member, ConstraintLocator *locator) {
DeclNameRef member, bool alreadyDiagnosed,
ConstraintLocator *locator) {
return new (cs.getAllocator())
DefineMemberBasedOnUse(cs, baseType, member, locator);
DefineMemberBasedOnUse(cs, baseType, member, alreadyDiagnosed, locator);
}

AllowMemberRefOnExistential *
Expand Down
15 changes: 12 additions & 3 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -806,10 +806,19 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
Type BaseType;
DeclNameRef Name;

/// Whether or not the member error is already diagnosed. This can happen
/// when referencing an erroneous member, and the error is diagnosed at the
/// member declaration.
///
/// We still want to define erroneous members based on use in order to find
/// a solution through the new diagnostic infrastructure, but we don't
/// want to report a second error message.
bool AlreadyDiagnosed;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Would you mind adding a comment about why this is necessary for posterity?


DefineMemberBasedOnUse(ConstraintSystem &cs, Type baseType, DeclNameRef member,
ConstraintLocator *locator)
bool alreadyDiagnosed, ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::DefineMemberBasedOnUse, locator),
BaseType(baseType), Name(member) {}
BaseType(baseType), Name(member), AlreadyDiagnosed(alreadyDiagnosed) {}

public:
std::string getName() const override {
Expand All @@ -822,7 +831,7 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
bool diagnose(bool asNote = false) const override;

static DefineMemberBasedOnUse *create(ConstraintSystem &cs, Type baseType,
DeclNameRef member,
DeclNameRef member, bool alreadyDiagnosed,
ConstraintLocator *locator);
};

Expand Down
15 changes: 7 additions & 8 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6121,8 +6121,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
return formUnsolved();

case MemberLookupResult::ErrorAlreadyDiagnosed:
return SolutionKind::Error;

case MemberLookupResult::HasResults:
// Keep going!
break;
Expand Down Expand Up @@ -6198,8 +6196,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
return type;
});

auto *fix =
DefineMemberBasedOnUse::create(*this, baseTy, member, locator);
bool alreadyDiagnosed = (result.OverallResult ==
MemberLookupResult::ErrorAlreadyDiagnosed);
auto *fix = DefineMemberBasedOnUse::create(*this, baseTy, member,
alreadyDiagnosed, locator);
// Impact is higher if the base is expected to be inferred from context,
// because a failure to find a member ultimately means that base type is
// not a match in this case.
Expand All @@ -6208,9 +6208,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
if (recordFix(fix, impact))
return SolutionKind::Error;

// Allow member type to default to `Any` to make it possible to form
// solutions when contextual type of the result cannot be deduced e.g.
// `let _ = x.foo`.
// Record a hole for memberTy to make it possible to form solutions
// when contextual result type cannot be deduced e.g. `let _ = x.foo`.
if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
recordPotentialHole(memberTypeVar);

Expand Down Expand Up @@ -7860,7 +7859,7 @@ ConstraintSystem::simplifyDynamicCallableApplicableFnConstraint(
DeclNameRef memberName({ ctx, ctx.Id_dynamicallyCall, {argLabel} });

auto *fix = DefineMemberBasedOnUse::create(
*this, desugar2, memberName,
*this, desugar2, memberName, /*alreadyDiagnosed=*/false,
getConstraintLocator(loc, ConstraintLocator::DynamicCallable));

if (recordFix(fix))
Expand Down
3 changes: 2 additions & 1 deletion test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ struct Toe {
let toenail: Nail // expected-error {{use of undeclared type 'Nail'}}

func clip() {
toenail.inspect { x in
// FIXME: We shouldn't report this because toenail.inspect is a hole
toenail.inspect { x in // expected-error {{unable to infer closure return type; add explicit type to disambiguate}}
Copy link
Member Author

Choose a reason for hiding this comment

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

We can fix this by using applicable function constraint to bind hole types to the correct shape

toenail.inspect { y in }
}
}
Expand Down
1 change: 0 additions & 1 deletion test/Constraints/rdar46377919.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@ class Foo {

func foo() -> Foo {
return Foo(lhs: 2, rhs: 2)
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '<<error type>>'}}
}