Skip to content

Address typo in type inference #975

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
Nov 1, 2023
Merged

Address typo in type inference #975

merged 2 commits into from
Nov 1, 2023

Conversation

Nigel-Ecma
Copy link
Contributor

Fix for issue #787

The PR uses “corresponding parameter” rather than the “ith parameter" from #787. I think this is better, avoids HTML tags, and matches the use of corresponding parameter elsewhere; but either choice is valid.

Fix for issue #787

The PR uses “corresponding parameter” rather than the “ith parameter" from #787. I think this is better, avoids HTML tags, and matches the use of corresponding parameter elsewhere; but either choice is valid.
@Nigel-Ecma Nigel-Ecma added this to the Pre-C# 8.0 milestone Oct 30, 2023
@Nigel-Ecma Nigel-Ecma self-assigned this Oct 30, 2023
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

I suspect it has to be corresponding parameter, and that we should change the "Ti" part to "the type of the corresponding parameter" or similar.

Otherwise, using named arguments in a way such that the order of the arguments doesn't match the order of the parameters breaks things... unless there's a notional reordering earlier on in the proces, of course.

@RexJaeschke RexJaeschke removed their request for review October 30, 2023 11:10
Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

This LGTM

@Nigel-Ecma
Copy link
Contributor Author

I suspect it has to be corresponding parameter, and that we should change the "Ti" part to "the type of the corresponding parameter" or similar.

Otherwise, using named arguments in a way such that the order of the arguments doesn't match the order of the parameters breaks things... unless there's a notional reordering earlier on in the proces, of course.

You were not meant to notice that ;-) I too realised it hasn't been updated for named arguments but choose just to fix the typo you found. We can always address named arguments later, or do you want to do this now?

@jskeet
Copy link
Contributor

jskeet commented Nov 1, 2023

Let's discuss in the meeting - if someone has a suggestion for a quick way to do it properly, let's do that; otherwise, this is better than it was before.

@jskeet jskeet added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Nov 1, 2023
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Merging as better than what we've got.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants