Skip to content

SR-1255: Improve diagnostic when one of the parameters marked as autoclosure #5239

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.

Sign up for GitHub

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
Oct 14, 2016
Merged

SR-1255: Improve diagnostic when one of the parameters marked as autoclosure #5239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 16 additions & 4 deletions lib/Sema/CSDiag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,15 +1286,27 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
bool mismatchesAreNearMisses = true;

CalleeCandidateInfo::FailedArgumentInfo failureInfo;


// Local function which extracts type from the parameter container.
auto getType = [](const CallArgParam &param) -> Type {
// If parameter is marked as @autoclosure, we are
// only interested in it's resulting type.
if (param.isAutoClosure()) {
if (auto fnType = param.Ty->getAs<AnyFunctionType>())
return fnType->getResult();
}

return param.Ty;
};

for (unsigned i = 0, e = paramBindings.size(); i != e; ++i) {
// Bindings specify the arguments that source the parameter. The only case
// this returns a non-singular value is when there are varargs in play.
auto &bindings = paramBindings[i];
auto paramType = candArgs[i].Ty;
auto paramType = getType(candArgs[i]);

for (auto argNo : bindings) {
auto argType = actualArgs[argNo].Ty;
auto argType = getType(actualArgs[argNo]);
auto rArgType = argType->getRValueType();

// If the argument has an unresolved type, then we're not actually
Expand Down Expand Up @@ -5263,7 +5275,7 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
argType = tupleTy;
}
}

// Get the expression result of type checking the arguments to the call
// independently, so we have some idea of what we're working with.
//
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/bridging.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func rdar19836341(_ ns: NSString?, vns: NSString?) {
// <rdar://problem/20029786> Swift compiler sometimes suggests changing "as!" to "as?!"
func rdar20029786(_ ns: NSString?) {
var s: String = ns ?? "str" as String as String // expected-error{{cannot convert value of type 'NSString?' to expected argument type 'String?'}}
var s2 = ns ?? "str" as String as String // expected-error {{binary operator '??' cannot be applied to operands of type 'NSString?' and 'String'}} expected-note{{}}
var s2 = ns ?? "str" as String as String // expected-error {{cannot convert value of type 'String' to expected argument type 'NSString'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

As with the example in expr/expressions.swift, this seems like a regression.

Copy link
Contributor Author

@xedin xedin Oct 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

As a user I think it makes more sense to get an error about "str" as String producing unexpected type of "String" instead of "NSString" which would much an "NSString?" in that case, because original error is not every helpful to figure out what is actually wrong...

Here is the output of the new diagnostic:

<REPL Input>:2:44: error: cannot convert value of type 'String' to expected argument type 'NSString'
            var s2 = ns ?? "str" as String as String
                           ~~~~~~~~~~~~~~~~^~~~~~~~~
                                                     as NSString


let s3: NSString? = "str" as String? // expected-error {{cannot convert value of type 'String?' to specified type 'NSString?'}}

Expand Down
14 changes: 10 additions & 4 deletions test/Constraints/diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,7 @@ func r24251022() {
func overloadSetResultType(_ a : Int, b : Int) -> Int {
// https://twitter.com/_jlfischer/status/712337382175952896
// TODO: <rdar://problem/27391581> QoI: Nonsensical "binary operator '&&' cannot be applied to two 'Bool' operands"
return a == b && 1 == 2 // expected-error {{binary operator '&&' cannot be applied to two 'Bool' operands}}
// expected-note @-1 {{expected an argument list of type '(Bool, @autoclosure () throws -> Bool)'}}
return a == b && 1 == 2 // expected-error {{cannot convert return expression of type 'Bool' to return type 'Int'}}
}

// <rdar://problem/21523291> compiler error message for mutating immutable field is incorrect
Expand Down Expand Up @@ -733,9 +732,8 @@ class Foo23752537 {
extension Foo23752537 {
func isEquivalent(other: Foo23752537) {
// TODO: <rdar://problem/27391581> QoI: Nonsensical "binary operator '&&' cannot be applied to two 'Bool' operands"
// expected-error @+1 {{binary operator '&&' cannot be applied to two 'Bool' operands}}
// expected-error @+1 {{unexpected non-void return value in void function}}
return (self.title != other.title && self.message != other.message)
// expected-note @-1 {{expected an argument list of type '(Bool, @autoclosure () throws -> Bool)'}}
}
}

Expand Down Expand Up @@ -815,3 +813,11 @@ r27212391(a: 1, 3, y: 5) // expected-error {{missing argument label 'x' in ca
r27212391(1, x: 3, y: 5) // expected-error {{missing argument label 'a' in call}}
r27212391(a: 1, y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}}
r27212391(a: 1, 3, x: 5) // expected-error {{argument 'x' must precede unnamed argument #2}}

// SR-1255
func foo1255_1() {
return true || false // expected-error {{unexpected non-void return value in void function}}
}
func foo1255_2() -> Int {
return true || false // expected-error {{cannot convert return expression of type 'Bool' to return type 'Int'}}
}
3 changes: 1 addition & 2 deletions test/expr/expressions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,8 +735,7 @@ func testNilCoalescePrecedence(cond: Bool, a: Int?, r: CountableClosedRange<Int>

// ?? should have lower precedence than range and arithmetic operators.
let r1 = r ?? (0...42) // ok
let r2 = (r ?? 0)...42 // not ok: expected-error {{binary operator '??' cannot be applied to operands of type 'CountableClosedRange<Int>?' and 'Int'}}
// expected-note @-1 {{overloads for '??' exist with these partially matching parameter lists:}}
let r2 = (r ?? 0)...42 // not ok: expected-error {{cannot convert value of type 'Int' to expected argument type 'CountableClosedRange<Int>'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really seem like an improvement, but I'm not sure if it's really that bad if we have the caret in the right place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is complete output of the diagnostic (caret is at the right place):

(swift) func testNilCoalescePrecedence(cond: Bool, a: Int?, r: CountableClosedRange<Int>?) {
          let r1 = r ?? (0...42)
          let r2 = (r ?? 0)...42
        }
<REPL Input>:3:18: error: cannot convert value of type 'Int' to expected argument type 'CountableClosedRange<Int>'
  let r2 = (r ?? 0)...42
                 ^

let r3 = r ?? 0...42 // parses as the first one, not the second.


Expand Down