Skip to content

[Diagnostics] Ported tuple mismatch diagnostic to new diagnostic framework #26391

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 2 commits into from
Aug 2, 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
14 changes: 6 additions & 8 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1147,10 +1147,9 @@ bool FailureDiagnosis::diagnoseGeneralConversionFailure(Constraint *constraint){
if (auto fromTT = fromType->getAs<TupleType>())
if (auto toTT = toType->getAs<TupleType>()) {
if (fromTT->getNumElements() != toTT->getNumElements()) {
diagnose(anchor->getLoc(), diag::tuple_types_not_convertible_nelts,
fromTT, toTT)
.highlight(anchor->getSourceRange());
return true;
auto failure = TupleContextualFailure(anchor, CS, fromTT, toTT,
CS.getConstraintLocator(expr));
return failure.diagnoseAsError();
}

SmallVector<TupleTypeElt, 4> FromElts;
Expand All @@ -1166,10 +1165,9 @@ bool FailureDiagnosis::diagnoseGeneralConversionFailure(Constraint *constraint){
// then we have a type error.
if (computeTupleShuffle(TEType->castTo<TupleType>()->getElements(),
toTT->getElements(), sources)) {
diagnose(anchor->getLoc(), diag::tuple_types_not_convertible,
fromTT, toTT)
.highlight(anchor->getSourceRange());
return true;
auto failure = TupleContextualFailure(anchor, CS, fromTT, toTT,
CS.getConstraintLocator(expr));
return failure.diagnoseAsError();
}
}

Expand Down
8 changes: 8 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1887,6 +1887,14 @@ void ContextualFailure::tryComputedPropertyFixIts(Expr *expr) const {
}
}

bool TupleContextualFailure::diagnoseAsError() {
auto diagnostic = isNumElementsMismatch()
? diag::tuple_types_not_convertible_nelts
: diag::tuple_types_not_convertible;
emitDiagnostic(getAnchor()->getLoc(), diagnostic, getFromType(), getToType());
return true;
}

bool AutoClosureForwardingFailure::diagnoseAsError() {
auto path = getLocator()->getPath();
assert(!path.empty());
Expand Down
17 changes: 17 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,23 @@ class ContextualFailure : public FailureDiagnostic {
void tryComputedPropertyFixIts(Expr *expr) const;
};

/// Diagnose mismatches relating to tuple destructuring.
class TupleContextualFailure final : public ContextualFailure {
public:
TupleContextualFailure(Expr *root, ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
: ContextualFailure(root, cs, lhs, rhs, locator) {}

bool diagnoseAsError() override;

bool isNumElementsMismatch() const {
auto lhsTy = getFromType()->castTo<TupleType>();
auto rhsTy = getToType()->castTo<TupleType>();
assert(lhsTy && rhsTy);
return lhsTy->getNumElements() != rhsTy->getNumElements();
}
};

/// Diagnose situations when @autoclosure argument is passed to @autoclosure
/// parameter directly without calling it first.
class AutoClosureForwardingFailure final : public FailureDiagnostic {
Expand Down
14 changes: 14 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,20 @@ ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
}

bool AllowTupleTypeMismatch::diagnose(Expr *root, bool asNote) const {
auto failure = TupleContextualFailure(
root, getConstraintSystem(), getFromType(), getToType(), getLocator());
return failure.diagnose(asNote);
}

AllowTupleTypeMismatch *
AllowTupleTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator) {
assert(lhs->is<TupleType>() && rhs->is<TupleType>() &&
"lhs and rhs must be tuple types");
return new (cs.getAllocator()) AllowTupleTypeMismatch(cs, lhs, rhs, locator);
}

bool GenericArgumentsMismatch::diagnose(Expr *root, bool asNote) const {
auto failure = GenericArgumentsMismatchFailure(root, getConstraintSystem(),
getActual(), getRequired(),
Expand Down
24 changes: 24 additions & 0 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ enum class FixKind : uint8_t {
/// referenced constructor must be required.
AllowInvalidInitRef,

/// Allow a tuple to be destructured with mismatched arity, or mismatched
/// types.
AllowTupleTypeMismatch,

/// Allow an invalid member access on a value of protocol type as if
/// that protocol type were a generic constraint requiring conformance
/// to that protocol.
Expand Down Expand Up @@ -494,6 +498,9 @@ class ContextualMismatch : public ConstraintFix {
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::ContextualMismatch, locator), LHS(lhs),
RHS(rhs) {}
ContextualMismatch(ConstraintSystem &cs, FixKind kind, Type lhs, Type rhs,
ConstraintLocator *locator)
: ConstraintFix(cs, kind, locator), LHS(lhs), RHS(rhs) {}

public:
std::string getName() const override { return "fix contextual mismatch"; }
Expand Down Expand Up @@ -862,6 +869,23 @@ class AllowInvalidInitRef final : public ConstraintFix {
ConstraintLocator *locator);
};

class AllowTupleTypeMismatch final : public ContextualMismatch {
AllowTupleTypeMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
: ContextualMismatch(cs, FixKind::AllowTupleTypeMismatch, lhs, rhs,
locator) {}

public:
static AllowTupleTypeMismatch *create(ConstraintSystem &cs, Type lhs,
Type rhs, ConstraintLocator *locator);

std::string getName() const override {
return "fix tuple mismatches in type and arity";
}

bool diagnose(Expr *root, bool asNote = false) const override;
};

class AllowMutatingMemberOnRValueBase final : public AllowInvalidMemberRef {
AllowMutatingMemberOnRValueBase(ConstraintSystem &cs, Type baseType,
ValueDecl *member, DeclName name,
Expand Down
51 changes: 51 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2382,6 +2382,13 @@ bool ConstraintSystem::repairFailures(
getConstraintLocator(locator));
conversionsOrFixes.push_back(fix);
}

if (purpose == CTP_Initialization && lhs->is<TupleType>() &&
rhs->is<TupleType>()) {
auto *fix = AllowTupleTypeMismatch::create(*this, lhs, rhs,
getConstraintLocator(locator));
conversionsOrFixes.push_back(fix);
}
break;
}

Expand Down Expand Up @@ -2413,6 +2420,11 @@ bool ConstraintSystem::repairFailures(
conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
*this, lhs, rhs, getConstraintLocator(locator)));
}
if (lhs->is<TupleType>() && rhs->is<TupleType>()) {
auto *fix = AllowTupleTypeMismatch::create(*this, lhs, rhs,
getConstraintLocator(locator));
conversionsOrFixes.push_back(fix);
}
break;
}

Expand Down Expand Up @@ -6951,6 +6963,45 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return matchTypes(type1, type2, matchKind, subflags, locator);
}

case FixKind::AllowTupleTypeMismatch: {
auto lhs = type1->castTo<TupleType>();
auto rhs = type2->castTo<TupleType>();
// Create a new tuple type the size of the smaller tuple with elements
// from the larger tuple whenever either side contains a type variable.
// For example (A, $0, B, $2) and (X, Y, $1) produces: (X, $0, B).
// This allows us to guarentee that the types will match, and all
// type variables will get bound to something as long as we default
// excess types in the larger tuple to Any. In the prior example,
// when the tuples (X, Y, $1) and (X, $0, B) get matched, $0 is equated
// to Y, $1 is equated to B, and $2 is defaulted to Any.
auto lhsLarger = lhs->getNumElements() >= rhs->getNumElements();
auto larger = lhsLarger ? lhs : rhs;
auto smaller = lhsLarger ? rhs : lhs;
llvm::SmallVector<TupleTypeElt, 4> newTupleTypes;

for (unsigned i = 0; i < larger->getNumElements(); ++i) {
auto largerElt = larger->getElement(i);
if (i < smaller->getNumElements()) {
auto smallerElt = smaller->getElement(i);
if (largerElt.getType()->isTypeVariableOrMember() ||
smallerElt.getType()->isTypeVariableOrMember())
newTupleTypes.push_back(largerElt);
else
newTupleTypes.push_back(smallerElt);
} else {
if (largerElt.getType()->isTypeVariableOrMember())
addConstraint(ConstraintKind::Defaultable, largerElt.getType(),
getASTContext().TheAnyType,
getConstraintLocator(locator));
}
}
auto matchingType =
TupleType::get(newTupleTypes, getASTContext())->castTo<TupleType>();
if (recordFix(fix))
return SolutionKind::Error;
return matchTupleTypes(matchingType, smaller, matchKind, subflags, locator);
}

case FixKind::InsertCall:
case FixKind::RemoveReturn:
case FixKind::AddConformance:
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/sr10728.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ struct S: P {
typealias R = T3

static let foo: (T1, (R) -> T2) = bind()
// expected-error@-1 {{cannot convert value of type '(T1, (S.R) -> T3)' (aka '(Int, (Bool) -> Bool)') to specified type '(T1, (S.R) -> T2)' (aka '(Int, (Bool) -> Float)')}}
// expected-error@-1 {{tuple type '(T1, (S.R) -> T3)' (aka '(Int, (Bool) -> Bool)') is not convertible to tuple '(T1, (S.R) -> T2)' (aka '(Int, (Bool) -> Float)')}}
}
4 changes: 2 additions & 2 deletions test/NameBinding/name-binding.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func test_varname_binding() {
var (d, e) = (c.1, c.0)
var ((), (g1, g2), h) = ((), (e, d), e)
var (j, k, l) = callee1()
var (m, n) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(_, _)', tuples have a different number of elements}}
var (o, p, q, r) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(_, _, _, _)', tuples have a different number of elements}}
var (m, n) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}
var (o, p, q, r) = callee1() // expected-error{{'(Int, Int, Int)' is not convertible to '(Int, Int, Int, Any)', tuples have a different number of elements}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice improvement :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice indeed!

}

//===----------------------------------------------------------------------===//
Expand Down
13 changes: 8 additions & 5 deletions test/expr/expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func funcdecl7(_ a: Int, b: (c: Int, d: Int), third: (c: Int, d: Int)) -> Int {

// Error recovery.
func testfunc2 (_: ((), Int) -> Int) -> Int {}
func makeTuple() -> (String, Int) { return ("foo", 42) }
func errorRecovery() {
testfunc2({ $0 + 1 }) // expected-error {{contextual closure type '((), Int) -> Int' expects 2 arguments, but 1 was used in closure body}}

Expand All @@ -149,12 +150,14 @@ func errorRecovery() {
var a: Int = .hello // expected-error {{type 'Int' has no member 'hello'}}
var b: union1 = .bar // ok
var c: union1 = .xyz // expected-error {{type 'union1' has no member 'xyz'}}
var d: (Int,Int,Int) = (1,2) // expected-error {{cannot convert value of type '(Int, Int)' to specified type '(Int, Int, Int)'}}
var e: (Int,Int) = (1, 2, 3) // expected-error {{cannot convert value of type '(Int, Int, Int)' to specified type '(Int, Int)'}}
var f: (Int,Int) = (1, 2, f : 3) // expected-error {{cannot convert value of type '(Int, Int, f: Int)' to specified type '(Int, Int)'}}
var d: (Int,Int,Int) = (1,2) // expected-error {{'(Int, Int)' is not convertible to '(Int, Int, Int)', tuples have a different number of elements}}
var e: (Int,Int) = (1, 2, 3) // expected-error {{'(Int, Int, Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}
var f: (Int,Int) = (1, 2, f : 3) // expected-error {{'(Int, Int, f: Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}

// <rdar://problem/22426860> CrashTracer: [USER] swift at …mous_namespace::ConstraintGenerator::getTypeForPattern + 698
var (g1, g2, g3) = (1, 2) // expected-error {{'(Int, Int)' is not convertible to '(_, _, _)', tuples have a different number of elements}}
var (g1, g2, g3) = (1, 2) // expected-error {{'(Int, Int)' is not convertible to '(Int, Int, Any)', tuples have a different number of elements}}
var (h1, h2) = (1, 2, 3) // expected-error {{'(Int, Int, Int)' is not convertible to '(Int, Int)', tuples have a different number of elements}}
var i: (Bool, Bool) = makeTuple() // expected-error {{tuple type '(String, Int)' is not convertible to tuple '(Bool, Bool)'}}
}

func acceptsInt(_ x: Int) {}
Expand Down Expand Up @@ -185,7 +188,7 @@ func test4() -> ((_ arg1: Int, _ arg2: Int) -> Int) {
func test5() {
let a: (Int, Int) = (1,2)
var
_: ((Int) -> Int, Int) = a // expected-error {{cannot convert value of type '(Int, Int)' to specified type '((Int) -> Int, Int)'}}
_: ((Int) -> Int, Int) = a // expected-error {{tuple type '(Int, Int)' is not convertible to tuple '((Int) -> Int, Int)'}}


let c: (a: Int, b: Int) = (1,2)
Expand Down