Skip to content
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

Add spec for extension GetEnumerator #3576

Merged
merged 2 commits into from
Jun 17, 2020

Conversation

333fred
Copy link
Member

@333fred 333fred commented Jun 16, 2020

Added the spec for extension GetEnumerator. The only change from #3194 was the additional line about the refkind in overload resolution.

@333fred
Copy link
Member Author

333fred commented Jun 16, 2020

/cc @YairHalberstadt

@YairHalberstadt
Copy link
Contributor

Should there be a section about async foreach?
Also there is a difference between extension foreach and instance foreach in that the former accepts params and optional arguments and the latter doesn't. I can't see any indication of that in the spec.

@333fred
Copy link
Member Author

333fred commented Jun 17, 2020

Should there be a section about async foreach?

Probably, will add.

Also there is a difference between extension foreach and instance foreach in that the former accepts params and optional arguments and the latter doesn't. I can't see any indication of that in the spec.

It doesn't need to be explicit. Overload resolution with a single argument will find methods with default parameters.

@YairHalberstadt
Copy link
Contributor

I would expect it to be explicit for instance GetEnumerator then.

Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

😷

@333fred
Copy link
Member Author

333fred commented Jun 17, 2020

I would expect it to be explicit for instance GetEnumerator then.

@YairHalberstadt thanks to some sleuthing from @gafter, there's yet another spec violation for compat that we kept: https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs#L1006-L1016. It absolutely should be including those parameters, and it does for async enumerable.

@333fred
Copy link
Member Author

333fred commented Jun 17, 2020

Note: the spec I'm referring to in the update is https://github.com/dotnet/roslyn/blob/master/docs/features/async-streams.md, not the one checked into csharplang. @jcouv has taken a work item to update the one in this repo, which is out of date in several aspects (including whether extension methods are permitted).

@333fred 333fred merged commit aa2ff4d into dotnet:master Jun 17, 2020
@333fred 333fred deleted the extension-getenumerator-spec branch June 17, 2020 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants