Collection expressions: use iteration type for convertibility of element types #71397
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
cston
force-pushed
the
use-iteration-type
branch
from
9484f9c
to
de50319
Compare
| { | ||
| elementType = default; | ||
| return CollectionExpressionTypeKind.ImplementsIEnumerableT; | ||
| } |
There was a problem hiding this comment.
Discussed offline:
It feels strange for GetCollectionExpressionTypeKind to introduce a concept of "element type" in some cases.
Could we align on the concept of iteration type for all kinds of collections and add the special cases to TryGetCollectionIterationType instead, or call TryGetCollectionIterationType here and rename elementType->iterationType?
That would simplify concepts and align with the spec better.
There was a problem hiding this comment.
Thanks for the suggestion. I agree, we should refactor these two methods, possibly moving calculation of iteration type to TryGetCollectionIterationType exclusively or having one method call the other. Let's refactor as necessary after completing other expected changes here.
| base(elementConversions) | ||
| { | ||
| Debug.Assert(collectionExpressionTypeKind != CollectionExpressionTypeKind.None); |
There was a problem hiding this comment.
consider adding assertion that elementType is not null (we already have such assertion in caller, but I think it would make sense here) #Closed
| { | ||
| private List<string> _list = new(); | ||
| public void Add(string s) { _list.Add(s); } | ||
| IEnumerator<string> IEnumerable<string>.GetEnumerator() => _list.GetEnumerator(); |
| { | ||
| private List<T> _list = new List<T>(); | ||
| public void Add(T t) | ||
| { | ||
| Console.WriteLine("Add {0}", t); | ||
| _list.Add(t); | ||
| } | ||
| IEnumerator<T> IEnumerable<T>.GetEnumerator() => _list.GetEnumerator(); |
There was a problem hiding this comment.
consider making the private IEnumerable.GetEnumerator() implementation below throw #Closed
jcouv
added
New Feature - Collection Expressions
and removed
untriaged
ghost
added this to the 
See corresponding spec change.
Relates to test plan #66418