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

Collection expressions: optimize calculation of iteration type #69676

Open
cston opened this issue Aug 23, 2023 · 3 comments
Open

Collection expressions: optimize calculation of iteration type #69676

cston opened this issue Aug 23, 2023 · 3 comments

Comments

@cston
Copy link
Member

cston commented Aug 23, 2023

Collection expressions use the iteration type of the target type for type inference, overload resolution, and verifying [CollectionBuilder] attributes. Currently, the iteration type calculation depends on the location where the collection type is used since iteration type may require looking for an extension GetEnumerator() method. That makes the calculation potentially expensive.

We should consider optimizing the calculation when possible. One possibility is to require types used for collection expressions to have an instance GetEnumerator() method rather than allowing extension methods. Then the calculation would be independent of the use-site, and the iteration type could be cached on the TypeSymbol directly. If extension methods are allowed, we could still cache the iteration type on the TypeSymbol for the instance method cases but fall back to binding if the iteration type is not cached.

See #69594 (comment).

@cston cston self-assigned this Aug 23, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 23, 2023
@cston cston added Feature - Collection Expressions and removed Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 23, 2023
@cston cston added this to the 17.8 milestone Aug 23, 2023
@CyrusNajmabadi
Copy link
Member

NOte: requiring instnace GetEnumerator would be weird especially as that means you could observe different behavior between foreaching something, versus collection-exprs. I'm wary of that.

@RikkiGibson
Copy link
Contributor

Could you give a specific scenario where you feel the behavior would be strange? I personally don't feel worried because instance GetEnumerator is always better than an extension. I def think that if you are applying CollectionBuilder, it is reasonable to be implementing GetEnumerator just once, on the same type which has the attribute.

@CyrusNajmabadi
Copy link
Member

it would certainly be a strange type if this mattered. By my general point is:

  1. for non-strange types, we have no problem. Because we'll find the instance GetEnumerator.
  2. for strange types, we shoudl make collection-expressions align with foreach.

If we don't follow '2', then we just end up with weird gotchas, which seems like something we'd like to avoid.

Given that we already have to do the extension work for any foreach... why not do this work for collection-exprs?

@arunchndr arunchndr modified the milestones: 17.8, Backlog Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants