Skip to content

Improve error messages when converting lambdas to expression trees#21380

Merged
jcouv merged 4 commits intodotnet:masterfrom
svick:delegate-expression-message
May 25, 2018
Merged

Improve error messages when converting lambdas to expression trees#21380
jcouv merged 4 commits intodotnet:masterfrom
svick:delegate-expression-message

Conversation

@svick
Copy link
Copy Markdown
Member

@svick svick commented Aug 8, 2017 •
edited
Loading

Fixes #18645.

The first commit adds a failing test.

The second commit fixes it by changing reporting of CS1593 for expression trees from e.g.:

Delegate 'Expression<Func<int, int>>' does not take 0 arguments

to:

Delegate 'Func<int, int>' does not take 0 arguments

The second commit also changes the wording of CS1661 from e.g.:

Cannot convert lambda expression to delegate type 'Expression<Func<int, int>>' because the parameter types do not match the delegate parameter types

to:

Cannot convert lambda expression to type 'Expression<Func<int, int>>' because the parameter types do not match the delegate parameter types

FYI @tmat

@svick svick changed the title Added failing test Improve error messages when converting lambdas to expression trees Aug 8, 2017
@Pilchie Pilchie added Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Sep 8, 2017
@Pilchie Pilchie requested a review from a team September 8, 2017 14:42
jcouv
jcouv approved these changes Sep 8, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM
It looks like there previously was not test coverage for that error. Thanks!

AlekseyTs
AlekseyTs previously approved these changes Sep 8, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM

TyOverby
TyOverby previously approved these changes Sep 20, 2017
View reviewed changes
@TyOverby
Copy link
Copy Markdown
Contributor

TyOverby commented Sep 20, 2017

@dotnet-bot test windows_release_unit32_prtest please

@jcouv
Copy link
Copy Markdown
Member

jcouv commented Oct 17, 2017

master is currently targeting 15.5 which only accepts very severe bugs (at this point in the release cycle). I'll rebase this PR to post-15.5 branch.

@jcouv jcouv changed the base branch from master to post-dev15.5-contrib October 17, 2017 17:36
@shyamnamboodiripad shyamnamboodiripad changed the base branch from post-dev15.5-contrib to master November 17, 2017 20:17
@jcouv
Copy link
Copy Markdown
Member

jcouv commented Apr 18, 2018

@dotnet-bot retest this please

@jcouv jcouv added this to the 15.8 milestone Apr 18, 2018
@jcouv jcouv self-assigned this Apr 18, 2018
@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 17, 2018

@svick It looks like master has drifted since the last time you refreshed this PR. I just want to give you a heads up that the bug bar is going to rise again for this milestone soon (around June 10th). I'll kick off new tests to get the current status.
If you intend to refresh the PR, please do so before then. Otherwise, we'll close as stale.
Thanks

@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 17, 2018

@dotnet-bot retest this please

@svick svick force-pushed the delegate-expression-message branch from 07b55ee to d87dd6a Compare May 18, 2018 10:12
@svick
Copy link
Copy Markdown
Member Author

svick commented May 18, 2018

@jcouv I have updated the PR.

jcouv
jcouv approved these changes May 18, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks

@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 18, 2018

@AlekseyTs This community PR was rebased since you had reviewed. Can you take another look so we can merge? Thanks

@AlekseyTs AlekseyTs dismissed stale reviews from TyOverby and themself May 21, 2018 22:39

Obsolete

AlekseyTs
AlekseyTs approved these changes May 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 4)

@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 22, 2018

test windows_debug_unit32_prtest please

@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 22, 2018

@jaredpar Please approve for ask-mode for 15.8. Thanks

@jaredpar
Copy link
Copy Markdown
Member

jaredpar commented May 22, 2018

approved

@jcouv jcouv merged commit acb2dd6 into dotnet:master May 25, 2018
@jcouv
Copy link
Copy Markdown
Member

jcouv commented May 25, 2018

Thanks @svick!

@svick svick deleted the delegate-expression-message branch May 25, 2018 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlekseyTs AlekseyTs AlekseyTs approved these changes

@jcouv jcouv jcouv approved these changes

+1 more reviewer

@TyOverby TyOverby TyOverby left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jcouv jcouv

Labels

Approved to merge Area-Compilers cla-already-signed Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Milestone

15.8

Development

Successfully merging this pull request may close these issues.

Inaccurate error message when expression tree lambda has the wrong number of arguments

7 participants

@svick @TyOverby @jcouv @jaredpar @AlekseyTs @Pilchie @dnfclas