Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

F# records in reference assembly (/ref folder) are missing properties except for the last record in file [regression with 7.0.100-rc.2.22477.23 SDK ] #14088

Closed
buvinghausen opened this issue Oct 11, 2022 · 16 comments · Fixed by #14093
Assignees
Labels
Area-Compiler Compiler-related issues which don't belong to other categories Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression release/17.5
Milestone

Comments

@buvinghausen
Copy link

buvinghausen commented Oct 11, 2022

After downloading and installing the latest 7.0.100-rc.2.22477.23 SDK I can no longer compile my C# libraries because if any code appears after the F# record declaration the compiler will throw a CS0117 error if you try to set a property on a CLIMutable record and/or a CS1061 error if you try to get any property on a record from C#.

Repro steps

Provide the steps required to reproduce the problem:

  1. Create an F# class library with 2 record types
  2. Create a C# console app that references the F# library
  3. Initialize the first record declaration and attempt to access the properties in the C# console

Clone here to speed up the repro

Expected behavior

Expect to be able to still use F# records in C# after F# code appears after them

Actual behavior

CS0117 if you attempt to set/initialize an F# record property where F# code appears after the record declaration
CS1061 if you attempt to get the value from an F# record property where F# code appears after the record declaration

Known workarounds

<ProduceReferenceAssembly>false</ProduceReferenceAssembly>

Related information

7.0.100-rc.2.22477.23 SDK on both Windows & Ubuntu

@T-Gro T-Gro added Regression Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Area-Compiler Compiler-related issues which don't belong to other categories and removed Needs-Triage labels Oct 12, 2022
@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

The properties (in the C# view of things) are missing alltogether:

image

@vzarytovskii
Copy link
Member

The properties (in the C# view of things) are missing alltogether:

image

What's in the IL?

@vzarytovskii
Copy link
Member

One thing rc2 has on by default is producing refassemblies for net7 projects, this might be it.

We might want to roll it back until fully tested.

@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

A few more hints:

When reading the properties of F# types via reflection, they are all there and public.
When loading the F# .dll via ildasm, the generated IL for both records is the same.
This exhibits itself in both Debug and Release.
The hypothesis of @buvinghausen confirmed: When I added more records, it is always the last record in the file which works.

@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

Indeed, the .dll within /ref folder is missing the properties for all but the last record:

image

@T-Gro T-Gro changed the title Issue with F# Records when compiling with 7.0.100-rc.2.22477.23 SDK F# records in reference assembly (/ref folder) are missing properties except for the last record in file [regression with 7.0.100-rc.2.22477.23 SDK ] Oct 12, 2022
@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

Workaround

To overcome this, please disable generation of F# reference assemblies (is on by default). That is, put the following in the .fsproj.
Note that the files are still cached and ref from the obj/Debug/ref folder, so I had to close VS, delete bin and obj, and then reopen.

After that, the project builds and runs.

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <GenerateDocumentationFile>true</GenerateDocumentationFile>
    <ProduceReferenceAssembly>false</ProduceReferenceAssembly>
  </PropertyGroup>

@vzarytovskii
Copy link
Member

vzarytovskii commented Oct 12, 2022

@T-Gro can you take a look at it please?

Ping me on teams if you'll need need help with figuring out how refassembly generation works.

It seems we generate getter and setter but not property itself.

Update: this is pr which introduces it #12334 I've been finishing it

@T-Gro T-Gro self-assigned this Oct 12, 2022
@vzarytovskii
Copy link
Member

canGenPropertyDef in ilwrite.fs is the place where we decide whether we want to emit property or not.

@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

Will cover this, thanks for the tips @vzarytovskii

@T-Gro
Copy link
Member

T-Gro commented Oct 12, 2022

The issue is curious at the IL gen level:

It only occurs when I have records with equaly-named properties.
What happens is that the properties in generated IL code get assigned to the last class in file, from all records across the module.

Even the screenshot above makes this clear - I had three records, and it generated each prop three times for the very last record.

@vzarytovskii
Copy link
Member

The issue is curious at the IL gen level:

It only occurs when I have records with equaly-named properties.
What happens is that the properties in generated IL code get assigned to the last class in file, from all records across the module.

Even the screenshot above makes this clear - I had three records, and it generated each prop three times for the very last record.

Huh, that's interesting, wondering if it's because we don't take types into account when we decide how to generate them?

@vzarytovskii
Copy link
Member

vzarytovskii commented Oct 12, 2022

@buvinghausen while we're investigating, setting ProduceReferenceAssembly msbuild property to false can be used as a workaround. Sorry for the inconvenience.

@buvinghausen
Copy link
Author

Thank you @vzarytovskii & @T-Gro for looking into this and providing insights. I can confirm the workaround does indeed get the compile errors to go away and I can proceed with testing our stack on RC2. Cheers.

@vzarytovskii
Copy link
Member

The fix should be in the next released of 17.4 and .net7

@vzarytovskii vzarytovskii added this to the Backlog milestone Oct 17, 2022
@buvinghausen
Copy link
Author

Thanks @vzarytovskii glad it will make it into the .NET 7.0 RTM cycle!

@vzarytovskii
Copy link
Member

This was fixed and inserted to 6.0 and 7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler Compiler-related issues which don't belong to other categories Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression release/17.5
Projects
Archived in project
3 participants