Skip to content

Reduce the number of captured name resolution environments in unions and records - #15549

Merged
vzarytovskii merged 8 commits into
dotnet:mainfrom
kerams:tcenv
Jul 20, 2023
Merged

Reduce the number of captured name resolution environments in unions and records#15549
vzarytovskii merged 8 commits into
dotnet:mainfrom
kerams:tcenv

Conversation

@kerams

@kerams kerams commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

Instead of incrementally inserting an environment for each field/case, we create only one for the entire representation scope as these entities cannot refer to each other.

Also fixes #13076.

Before

devenv_Tw2UxfsBOj

After

devenv_pyvfqBRRXZ

The difference in SyntaxTree.fs is 708 vs 1,054.

@kerams
kerams requested a review from a team as a code owner July 4, 2023 15:43
Comment thread src/Compiler/Checking/CheckDeclarations.fs
@kerams

kerams commented Jul 4, 2023

Copy link
Copy Markdown
Contributor Author

Can't run the failing test locally because I'm getting TypeInitializationException since the .NET 8 SDK switch.

Doesn't this look right though?

image

@edgarfgp

edgarfgp commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

Can't run the failing test locally because I'm getting TypeInitializationException since the .NET 8 SDK switch.

Doesn't this look right though?

image

Had a similar issues and I fixed by git cleaning it and then only build the FSharp.Compiler.Service.sln alone without using the build.cmd part . Hope it helps

Comment thread tests/FSharp.Compiler.Service.Tests/TooltipTests.fs
@vzarytovskii

Copy link
Copy Markdown
Member

Can't run the failing test locally because I'm getting TypeInitializationException since the .NET 8 SDK switch.

Doesn't this look right though?

image

Uh. That doesn't sound right. Do you have the specific issue?

@vzarytovskii
vzarytovskii enabled auto-merge (squash) July 20, 2023 14:36
@vzarytovskii
vzarytovskii merged commit 2c6344d into dotnet:main Jul 20, 2023
@kerams
kerams deleted the tcenv branch July 20, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Name resolution environment in record field does not contain generic type

5 participants