For tuple types, NamedTypeSymbol.GetFieldsToEmit API should return symbols for tuple underlying fields. #43552
Conversation
…mbols for tuple underlying fields. Fixes dotnet#43524. Related to dotnet#43549.
|
@jcouv, @dotnet/roslyn-compiler Please review |
| @@ -889,6 +889,9 @@ internal override IEnumerable<FieldSymbol> GetFieldsToEmit() | |||
| if ((object)associatedField != null) | |||
| { | |||
| Debug.Assert((object)associatedField.AssociatedSymbol != null); | |||
|
|
|||
| associatedField = associatedField.TupleUnderlyingField ?? associatedField; | |||
There was a problem hiding this comment.
This pattern pops up a bit. Is it worth adding a new member like TulpeUnderlyingFieldOrSelf to encourage this as a best practice going forward?
There was a problem hiding this comment.
This pattern pops up a bit. Is it worth adding a new member like TulpeUnderlyingFieldOrSelf to encourage this as a best practice going forward?
Are you suggesting to add one? I didn't get an urge to add one.
There was a problem hiding this comment.
Reading the PR I was wondering to myself:
How will future developers know this is an important problem? If we had
TupleUnderlyingFieldOrSelfmaybe it would be a clue that this is important.
I don't feel strongly about this. It's just a thought I had reading through this PR.
There was a problem hiding this comment.
How will future developers know this is an important problem?
I guess I am not sure what is the problem to be aware of. There is an API TupleUnderlyingField, it is docummented to return null in some scenarios. What is there to miss or be confused about?
There was a problem hiding this comment.
Mostly that in the case the return is null then using this is the generally the correct fallback. That can also be inferred by looking through these cases.
As I said though, this isn't a big concern. Mostly just thinking out loud as I'm reading the PR.
|
@jcouv Please review |
|
@jcouv Please review, need a second sign-off |
ghost
added this to the 
Fixes #43524.
Related to #43549.