Skip to content

[ConstraintSystem] Detect and diagnose inability to infer type of closure parameter(s) #31809

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 7 commits into from
May 15, 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
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ ERROR(no_candidates_match_result_type,none,
"no '%0' candidates produce the expected contextual result type %1",
(StringRef, Type))

ERROR(cannot_infer_closure_parameter_type,none,
"unable to infer type of a closure parameter %0 in the current context",
(StringRef))
ERROR(cannot_infer_closure_type,none,
"unable to infer closure type in the current context", ())
ERROR(cannot_infer_closure_result_type,none,
Expand Down
25 changes: 10 additions & 15 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1087,32 +1087,27 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
// resolved and had to be bound to a placeholder "hole" type.
cs.increaseScore(SK_Hole);

ConstraintFix *fix = nullptr;
if (auto *GP = TypeVar->getImpl().getGenericParameter()) {
auto path = dstLocator->getPath();
// Drop `generic parameter` locator element so that all missing
// generic parameters related to the same path can be coalesced later.
auto *fix = DefaultGenericArgument::create(
fix = DefaultGenericArgument::create(
cs, GP,
cs.getConstraintLocator(dstLocator->getAnchor(), path.drop_back()));
if (cs.recordFix(fix))
return true;
} else if (TypeVar->getImpl().isClosureParameterType()) {
fix = SpecifyClosureParameterType::create(cs, dstLocator);
} else if (TypeVar->getImpl().isClosureResultType()) {
auto *fix = SpecifyClosureReturnType::create(
cs, TypeVar->getImpl().getLocator());
if (cs.recordFix(fix))
return true;
fix = SpecifyClosureReturnType::create(cs, dstLocator);
} else if (srcLocator->getAnchor() &&
isExpr<ObjectLiteralExpr>(srcLocator->getAnchor())) {
auto *fix = SpecifyObjectLiteralTypeImport::create(
cs, TypeVar->getImpl().getLocator());
if (cs.recordFix(fix))
return true;
fix = SpecifyObjectLiteralTypeImport::create(cs, dstLocator);
} else if (srcLocator->isKeyPathRoot()) {
auto *fix =
SpecifyKeyPathRootType::create(cs, TypeVar->getImpl().getLocator());
if (cs.recordFix(fix))
return true;
fix = SpecifyKeyPathRootType::create(cs, dstLocator);
}

if (fix && cs.recordFix(fix))
return true;
}
}

Expand Down
75 changes: 75 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
using namespace swift;
using namespace constraints;

static bool hasFixFor(const Solution &solution, ConstraintLocator *locator) {
return llvm::any_of(solution.Fixes, [&locator](const ConstraintFix *fix) {
return fix->getLocator() == locator;
});
}

FailureDiagnostic::~FailureDiagnostic() {}

bool FailureDiagnostic::diagnose(bool asNote) {
Expand Down Expand Up @@ -6125,6 +6131,75 @@ bool MissingContextualBaseInMemberRefFailure::diagnoseAsError() {
return true;
}

bool UnableToInferClosureParameterType::diagnoseAsError() {
auto *closure = castToExpr<ClosureExpr>(getRawAnchor());

// Let's check whether this closure is an argument to
// a call which couldn't be properly resolved e.g.
// missing member or invalid contextual reference and
// if so let's not diagnose this problem because main
// issue here is inability to establish context for
// closure inference.
//
// TODO(diagnostics): Once we gain an ability to determine
// originating source of type holes this check could be
// significantly simplified.
{
auto &solution = getSolution();

// If there is a contextual mismatch associated with this
// closure, let's not diagnose any parameter type issues.
if (hasFixFor(solution, getConstraintLocator(
closure, LocatorPathElt::ContextualType())))
return false;

if (auto *parentExpr = findParentExpr(closure)) {
while (parentExpr &&
(isa<TupleExpr>(parentExpr) || isa<ParenExpr>(parentExpr))) {
parentExpr = findParentExpr(parentExpr);
}

if (parentExpr) {
// Missing or invalid member reference in call.
if (auto *AE = dyn_cast<ApplyExpr>(parentExpr)) {
if (getType(AE->getFn())->isHole())
return false;
}

// Any fix anchored on parent expression makes it unnecessary
// to diagnose unability to infer parameter type because it's
// an indication that proper context couldn't be established to
// resolve the closure.
ASTNode parentNode(parentExpr);
if (llvm::any_of(solution.Fixes,
[&parentNode](const ConstraintFix *fix) -> bool {
return fix->getAnchor() == parentNode;
}))
return false;
}
}
}

auto paramIdx = getLocator()
->castLastElementTo<LocatorPathElt::TupleElement>()
.getIndex();

auto *PD = closure->getParameters()->get(paramIdx);

llvm::SmallString<16> id;
llvm::raw_svector_ostream OS(id);

if (PD->isAnonClosureParam()) {
OS << "$" << paramIdx;
} else {
OS << "'" << PD->getParameterName() << "'";
}
Comment on lines +6183 to +6196
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be using the new getName method defined below?

Copy link
Member

Choose a reason for hiding this comment

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

SpecifyClosureParameterType::getName is the name of the constraint fix, used in the -debug-constraints output


auto loc = PD->isAnonClosureParam() ? getLoc() : PD->getLoc();
emitDiagnosticAt(loc, diag::cannot_infer_closure_parameter_type, OS.str());
return true;
}

bool UnableToInferClosureReturnType::diagnoseAsError() {
auto *closure = castToExpr<ClosureExpr>(getRawAnchor());

Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,15 @@ class MissingContextualBaseInMemberRefFailure final : public FailureDiagnostic {
bool diagnoseAsError();
};

class UnableToInferClosureParameterType final : public FailureDiagnostic {
public:
UnableToInferClosureParameterType(const Solution &solution,
ConstraintLocator *locator)
: FailureDiagnostic(solution, locator) {}

bool diagnoseAsError();
};

class UnableToInferClosureReturnType final : public FailureDiagnostic {
public:
UnableToInferClosureReturnType(const Solution &solution,
Expand Down
33 changes: 33 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "ConstraintSystem.h"
#include "OverloadChoice.h"
#include "swift/AST/Expr.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Type.h"
#include "swift/AST/Types.h"
#include "swift/Basic/SourceManager.h"
Expand Down Expand Up @@ -1229,6 +1230,38 @@ SpecifyBaseTypeForContextualMember *SpecifyBaseTypeForContextualMember::create(
SpecifyBaseTypeForContextualMember(cs, member, locator);
}

std::string SpecifyClosureParameterType::getName() const {
std::string name;
llvm::raw_string_ostream OS(name);

auto *closure = castToExpr<ClosureExpr>(getAnchor());
auto paramLoc =
getLocator()->castLastElementTo<LocatorPathElt::TupleElement>();

auto *PD = closure->getParameters()->get(paramLoc.getIndex());

OS << "specify type for parameter ";
if (PD->isAnonClosureParam()) {
OS << "$" << paramLoc.getIndex();
} else {
OS << "'" << PD->getParameterName() << "'";
}

return OS.str();
}

bool SpecifyClosureParameterType::diagnose(const Solution &solution,
bool asNote) const {
UnableToInferClosureParameterType failure(solution, getLocator());
return failure.diagnose(asNote);
}

SpecifyClosureParameterType *
SpecifyClosureParameterType::create(ConstraintSystem &cs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) SpecifyClosureParameterType(cs, locator);
}

bool SpecifyClosureReturnType::diagnose(const Solution &solution,
bool asNote) const {
UnableToInferClosureReturnType failure(solution, getLocator());
Expand Down
19 changes: 18 additions & 1 deletion lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ enum class FixKind : uint8_t {
/// inferred and has to be specified explicitly.
SpecifyBaseTypeForContextualMember,

/// Type of the closure parameter used in the body couldn't be inferred
/// and has to be specified explicitly.
SpecifyClosureParameterType,

/// Closure return type has to be explicitly specified because it can't be
/// inferred in current context e.g. because it's a multi-statement closure.
SpecifyClosureReturnType,
Expand All @@ -253,7 +257,7 @@ enum class FixKind : uint8_t {

/// A warning fix that allows a coercion to perform a force-cast.
AllowCoercionToForceCast,

/// Allow key path root type mismatch when applying a key path that has a
/// root type not convertible to the type of the base instance.
AllowKeyPathRootTypeMismatch,
Expand Down Expand Up @@ -1712,6 +1716,19 @@ class SpecifyBaseTypeForContextualMember final : public ConstraintFix {
create(ConstraintSystem &cs, DeclNameRef member, ConstraintLocator *locator);
};

class SpecifyClosureParameterType final : public ConstraintFix {
SpecifyClosureParameterType(ConstraintSystem &cs, ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::SpecifyClosureParameterType, locator) {}

public:
std::string getName() const;

bool diagnose(const Solution &solution, bool asNote = false) const;

static SpecifyClosureParameterType *create(ConstraintSystem &cs,
ConstraintLocator *locator);
};

class SpecifyClosureReturnType final : public ConstraintFix {
SpecifyClosureReturnType(ConstraintSystem &cs, ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::SpecifyClosureReturnType, locator) {}
Expand Down
6 changes: 5 additions & 1 deletion lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2191,8 +2191,12 @@ namespace {
auto declaredTy = param->getType();
externalType = CS.openUnboundGenericType(declaredTy, paramLoc);
} else {
// Let's allow parameters which haven't been explicitly typed
// to become holes by default, this helps in situations like
// `foo { a in }` where `foo` doesn't exist.
externalType = CS.createTypeVariable(
paramLoc, TVO_CanBindToInOut | TVO_CanBindToNoEscape);
paramLoc,
TVO_CanBindToInOut | TVO_CanBindToNoEscape | TVO_CanBindToHole);
}

closureParams.push_back(param->toFunctionParam(externalType));
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9609,6 +9609,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::AllowTupleSplatForSingleParameter:
case FixKind::AllowInvalidUseOfTrailingClosure:
case FixKind::AllowNonClassTypeToConvertToAnyObject:
case FixKind::SpecifyClosureParameterType:
case FixKind::SpecifyClosureReturnType:
case FixKind::AddQualifierToAccessTopLevelName:
llvm_unreachable("handled elsewhere");
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/ConstraintGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,10 @@ bool ConstraintGraph::contractEdges() {
if (isParamBindingConstraint && tyvar1->getImpl().canBindToInOut()) {
bool isNotContractable = true;
if (auto bindings = CS.getPotentialBindings(tyvar1)) {
// Holes can't be contracted.
if (bindings.IsHole)
continue;

for (auto &binding : bindings.Bindings) {
auto type = binding.BindingType;
isNotContractable = type.findIf([&](Type nestedType) -> bool {
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ class TypeVariableType::Implementation {
/// Determine whether this type variable represents a closure type.
bool isClosureType() const;

/// Determine whether this type variable represents one of the
/// parameter types associated with a closure.
bool isClosureParameterType() const;

/// Determine whether this type variable represents a closure result type.
bool isClosureResultType() const;

Expand Down
8 changes: 8 additions & 0 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ bool TypeVariableType::Implementation::isClosureType() const {
return isExpr<ClosureExpr>(locator->getAnchor()) && locator->getPath().empty();
}

bool TypeVariableType::Implementation::isClosureParameterType() const {
if (!(locator && locator->getAnchor()))
return false;

return isExpr<ClosureExpr>(locator->getAnchor()) &&
locator->isLastElement<LocatorPathElt::TupleElement>();
}

bool TypeVariableType::Implementation::isClosureResultType() const {
if (!(locator && locator->getAnchor()))
return false;
Expand Down
20 changes: 19 additions & 1 deletion test/Constraints/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ struct S_3520 {
func sr3520_set_via_closure<S, T>(_ closure: (inout S, T) -> ()) {} // expected-note {{in call to function 'sr3520_set_via_closure'}}
sr3520_set_via_closure({ $0.number1 = $1 })
// expected-error@-1 {{generic parameter 'S' could not be inferred}}
// expected-error@-2 {{generic parameter 'T' could not be inferred}}
// expected-error@-2 {{unable to infer type of a closure parameter $1 in the current context}}

// SR-3073: UnresolvedDotExpr in single expression closure

Expand Down Expand Up @@ -1017,3 +1017,21 @@ func overloaded_with_default_and_autoclosure<T>(b: Int = 0, c: @escaping () -> T

overloaded_with_default_and_autoclosure { 42 } // Ok
overloaded_with_default_and_autoclosure(42) // Ok

// SR-12815 - `error: type of expression is ambiguous without more context` in many cases where methods are missing
func sr12815() {
let _ = { a, b in }
// expected-error@-1 {{unable to infer type of a closure parameter 'a' in the current context}}
// expected-error@-2 {{unable to infer type of a closure parameter 'b' in the current context}}

_ = .a { b in } // expected-error {{cannot infer contextual base in reference to member 'a'}}

struct S {}

func test(s: S) {
S.doesntExist { b in } // expected-error {{type 'S' has no member 'doesntExist'}}
s.doesntExist { b in } // expected-error {{value of type 'S' has no member 'doesntExist'}}
s.doesntExist1 { v in } // expected-error {{value of type 'S' has no member 'doesntExist1'}}
.doesntExist2() { $0 }
}
}
5 changes: 2 additions & 3 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,7 @@ struct Toe {
let toenail: Nail // expected-error {{cannot find type 'Nail' in scope}}

func clip() {
// TODO(diagnostics): Solver should stop once it has detected that `toenail` doesn't exist and report that.
toenail.inspect { x in // expected-error {{type of expression is ambiguous without more context}}
toenail.inspect { x in
toenail.inspect { y in }
}
}
Expand Down Expand Up @@ -297,7 +296,7 @@ func r18800223(_ i : Int) {
}

// <rdar://problem/21883806> Bogus "'_' can only appear in a pattern or on the left side of an assignment" is back
_ = { $0 } // expected-error {{unable to infer closure type in the current context}}
_ = { $0 } // expected-error {{unable to infer type of a closure parameter $0 in the current context}}



Expand Down
8 changes: 4 additions & 4 deletions test/Sema/diag_ambiguous_overloads.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ fe(.baz) // expected-error {{reference to member 'baz' cannot be resolved withou
fe(.nope, .nyet) // expected-error {{type 'Int' has no member 'nope'}}
// expected-error@-1 {{reference to member 'nyet' cannot be resolved without a contextual type}}

func fg<T>(_ f: (T) -> T) -> Void {} // expected-note {{in call to function 'fg'}}
fg({x in x}) // expected-error {{generic parameter 'T' could not be inferred}}
func fg<T>(_ f: (T) -> T) -> Void {}
fg({x in x}) // expected-error {{unable to infer type of a closure parameter 'x' in the current context}}


struct S {
func f<T>(_ i: (T) -> T, _ j: Int) -> Void {} // expected-note {{in call to function 'f'}}
func f<T>(_ i: (T) -> T, _ j: Int) -> Void {}
func f(_ d: (Double) -> Double) -> Void {}
func test() -> Void {
f({x in x}, 2) // expected-error {{generic parameter 'T' could not be inferred}}
f({x in x}, 2) // expected-error {{unable to infer type of a closure parameter 'x' in the current context}}
}

func g<T>(_ a: T, _ b: Int) -> Void {}
Expand Down
4 changes: 2 additions & 2 deletions test/decl/typealias/generic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ typealias E<T1, T2> = Int // expected-note {{generic type 'E' declared here}}
// expected-note@-1 {{'T1' declared as parameter to type 'E'}}
// expected-note@-2 {{'T2' declared as parameter to type 'E'}}

typealias F<T1, T2> = (T1) -> T2 // expected-note {{'T1' declared as parameter to type 'F'}}
typealias F<T1, T2> = (T1) -> T2

// Type alias of type alias.
typealias G<S1, S2> = A<S1, S2>
Expand All @@ -94,7 +94,7 @@ let _ : D<Int, Int, Float> = D(a: 1, b: 2)

let _ : F = { (a : Int) -> Int in a } // Infer the types of F

let _ : F = { a in a } // expected-error {{generic parameter 'T1' could not be inferred}}
let _ : F = { a in a } // expected-error {{unable to infer type of a closure parameter 'a' in the current context}}

_ = MyType(a: "foo", b: 42)
_ = A(a: "foo", b: 42)
Expand Down
Loading