Skip to content

Ensure specialized signatures are always at the top when performing call candidate resolution#1562

Merged
JsonFreeman merged 2 commits into
microsoft:masterfrom
csnover:fix-1133
Jan 14, 2015
Merged

Ensure specialized signatures are always at the top when performing call candidate resolution#1562
JsonFreeman merged 2 commits into
microsoft:masterfrom
csnover:fix-1133

Conversation

@csnover

@csnover csnover commented Dec 26, 2014

Copy link
Copy Markdown
Contributor

This patch ensures specialized signatures are always at the top when performing call candidate resolution. It also includes an update to the compiler/inheritedOverloadedSpecializedSignatures test to add the failing merge condition, and an update to fourslash/overloadOnConstCallSignature now that specialized signatures are ordered to the top of the candidate list.

Edit by @DanielRosenwasser: Fixes #1133

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does seem weird, but this might be a problem with the strategy for signature help. Ideally, you would see the string overload because it is the most general.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I personally feel like this updated behaviour is actually better, since it indicates to the author immediately that there are specialised signatures to pick from.

In any case, I am not sure that the extra complexity of doing something different would be worth the cost (it might be, if the only cost is to add a loop to skip hasStringLiteral signatures in this situation), but that is just my opinion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, in general the philosophy for signature help is to give the user a lens into the ordering of the signatures. In that respect I think seeing the specialized first is better. So I am good with the way you have it.

@DanielRosenwasser DanielRosenwasser changed the title Fix #1133 Ensure specialized signatures are always at the top when performing call candidate resolution Jan 8, 2015
@csnover

csnover commented Jan 9, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I will try to get these fixes to you over the weekend so you can do what you want to do next week.

@csnover

csnover commented Jan 12, 2015

Copy link
Copy Markdown
Contributor Author

OK, the feedback should all be addressed now. Let me know if you need anything else from me on this, and thanks again for the review.

@JsonFreeman

Copy link
Copy Markdown
Contributor

Yup, looks good, thanks Colin!

JsonFreeman added a commit that referenced this pull request Jan 14, 2015
Ensure specialized signatures are always at the top when performing call candidate resolution
@JsonFreeman
JsonFreeman merged commit 4a1851c into microsoft:master Jan 14, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interface combining with multiple interface inheritance is weird

2 participants