Skip to content

Conversation

@TIHan
Copy link
Contributor

@TIHan TIHan commented Aug 13, 2020

Resolves #9914

The best way to handle record labels and unions was adding a new field to name resolution to lookup the type instantiations based on TyconRef.

@TIHan TIHan requested a review from dsyme August 13, 2020 02:45
@TIHan
Copy link
Contributor Author

TIHan commented Aug 14, 2020

This is ready @cartermp @dsyme

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Changes seem straightforward, as do the tests. Are there any perf considerations for freshening a given record field reference? I don't have a good frame of reference for that, but I recall freshening being expensive for some things in a different PR and was curious.

@TIHan
Copy link
Contributor Author

TIHan commented Aug 15, 2020

As far as freshening and perf, we are not freshening any more than before so I'm not concerned.

@brettfo brettfo changed the base branch from master to main August 19, 2020 19:59
@cartermp cartermp merged commit 4d02627 into dotnet:main Aug 21, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* Fixed generic union type instantiation on open type

* Fixed record type instantiation. Added tests for pattern matching on union and records.

* Consolidating fix

* remove comment

* Renamed field and updated comment

* Minor updates
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.

Open type declaration, generic union and record should use their enclosing type instantiations

4 participants