Tuple and underlying type unification. Part 1. #11006
Conversation
No behavior changes intended.
- Include memebers of underlying type into a set of members returned by tuple type symbol. Properties and events are not supported at the moment. - Track declaration locations for tuple related symbols. - Ensure Rest extensions are tuples.
|
test prtest/lin/dbg/unit32 please |
|
test prtest/win/dbg/unit64 please |
|
test prtest/lin/dbg/unit32 please |
|
@dotnet/roslyn-infrastructure It looks like something is wrong with Linux machines: |
|
Is there a reason we are doing this in features/tuples instead of future? |
| /// <summary> | ||
| /// Represents a field of a tuple type (such as (int, byte).Item1) | ||
| /// that doesn't have a corresponding backing field within the tuple underlying type. | ||
| /// Created in response to an error condition. |
There was a problem hiding this comment.
Confused on this point. (int, byte).Item1 does have a backing field. Is there a subtle issue I'm missing?
There was a problem hiding this comment.
I am not sure, only you can tell. :-) Perhaps you can ask a more specific question.
There was a problem hiding this comment.
Does this type get instantiated for (int, byte).Item1? If so then I find the comment confusing because it has a real backing field: ValueTuple<int,byte>.Item1.
In reply to: 61766119 [](ancestors = 61766119)
There was a problem hiding this comment.
That is because you think we live in the perfect world. :-) However, we should be prepared that ValueTuple<,> doesn't have the Item1 field of the expected type, i.e. malformed. The comment specifically talks about an error situation. If you think it is not clear enough, please provide your wording and I will paste it in.
This is where I started working. Once I have all the pieces in place, I'll merge into future. |
| // do not lose the original element names in the literal if different from names in the target | ||
|
|
||
| // PROTOTYPE(tuples): Come back to this, what about locations? |
There was a problem hiding this comment.
If this will be targeted to the future branch, this needs to be resolved. #Resolved
There was a problem hiding this comment.
It is not targeted to the future.
|
👍 |
| NamedTypeSymbol currentLinkType = tupleType.UnderlyingTupleType; | ||
| if ((object)underlyingField == null) | ||
| { | ||
| // Use-site error must have been repoted elsewhere. |
There was a problem hiding this comment.
| return new TupleTypeSymbol(locations, tupleCompatibleType, elementLocations, elementNames, elementTypes); | ||
| } | ||
|
|
||
| private static NamedTypeSymbol EnsureRestExtensionsAreTuples(NamedTypeSymbol tupleCompatibleType) |
There was a problem hiding this comment.
Sure, I'll try to come up with something. I encourage you to provide an example of what you would put there to make sure your specific concern, if any, is addressed.
In reply to: 61807571 [](ancestors = 61807571)
|
LGTM |
|
test prtest/lin/dbg/unit32 please |
|
LGTM |
tuple type symbol. Properties and events are not supported at the moment.
It is probably better to review commit by commit.