Fix debugging of code using Anonymous records

[dsyme]

This fixes #6512, debugging of code using anonymous records

@cartermp Like #6606 this should go into the fixes ASAP

The problem was with the name of the type for anonymous records in generated .NET code had an extra ' character that was confusing the debuggers. Just removing the character is the best fix.

[cartermp]

The problem was with the name of the type for anonymous records in generated .NET code had an extra ' character that was confusing the debuggers. Just removing the character is the best fix.

Wow, interesting!

[matthid]

I think it is early enough that we can get away with this. But do I understand this correctly that this introduces a binary breaking change for users just by updating the compiler?

[cartermp]

Thanks for bringing it up, it's worth thinking about.

@dsyme would be the expert here but my gut feel is no. The stamp in an anonymous record is calculated with a hashing function and we don't guarantee binary compatibility for a given hash function.

Here's another take:

I now agree we don't need to be backwards compatible, and I've been quite surprised that the lack of determinism/stability for .NET hash codes (e.g. of strings) has not seemed to have been a problem in practice.

Still, I'm not particularly keen on changing things unless we really, really have to and we address a number of known issues simultaneously. An RFC would be needed I think... And several more people than just myself approve it....

[matthid]

Just to be clear: In the hashcode example we change behavior. In this particular PR we straight up break it and execution will fail at runtime as we basically renamed the type.

Example: #6479
After updating the compiler in the library project, I'd expect the console app to fail with a TypeNotFoundException (or something like that).
The 'fix' is to compile the console app with the new compiler as well.

By the way: Looking at the code and the error in #6479, I wouldn't be surprised if this PR fixes that issue as well.

[matthid]

(I know I'm potentially playing the 😈 here, but if a bunch of issues pop up because people started to depend on this we should at least have some idea how to move forward. Also those issues will probably pop up after some time...)

Personally I still think this PR is the right thing to do. But we should be ready to either tell everyone in the ecosystem that ' in the class-name are allowed or that this was a compiler bug breaking the IL spec and there is nothing else to do...

[cartermp]

After updating the compiler in the library project, I'd expect the console app to fail with a TypeNotFoundException (or something like that).

This is no different than if the underlying hash generated in the name had changed due to changing the hashing function (for some reason), no?

[matthid]

This is no different than if the underlying hash generated in the name had changed due to changing the hashing function (for some reason), no?

Yes and no. Yes because if the F# compiler depends on the hashcode this is indeed a unexpected sideeffect of the issue, which isn't mentioned in the issue. If there are such dependencies you should bring them up into the discussion (I haven't seen them but tbh. I focused on @dsyme comments).
No because we could just remove this dependency and make the compiler use it's own hash function and therefore keep the behavior stable (if we want/need). Which is not something we can do here. So I think it's unrelated.

[KevinRansom]

It’s a breaking change. But it’s one that we can live with, the work around is to recompile the library, and then life will be fine. The anonymous types feature first shipped in VS 16.0 which was I think less than 2 months ago. We will add a release note, describing the issue and the work around. Thanks Kevin

[dsyme]

Yes, it is a binary breaking change. Libraries compiled with a dev16.0 compiler that use public anonymous records and consumed with a dev16.1 compiler will experience a peverify/runtime failure. The library should be recompiled with a dev16.1 compiler.

@KevinRansom is correct, we should just make this change. Using anonymous records is rare at this stage, and using them in the API of public-facing libraries (e.g. ones pushed to nuget) even rarer, and consuming those libraries with a later compiler even rarer still. And the adjustment needed to recompile and republish the library is fairly simple.

We should in future be much surer to check scenarios such as debugging (and not just rely on the author of the PR to do it....).

[JaggerJo]

Is there a schedule on when to expect this fix to appear as a VS Update / in the dotnet core sdk ?

[cartermp]

@JaggerJo The 16.1 VS release and associated .NET Core SDK.

[lfr]

@cartermp I just installed VS 16.1.1 today and rebuilt everything (even manually deleted obj folders) and I'm still experiencing this issue, have I done something wrong?

[cartermp]

I confirmed that this fix is in VS 16.2 and it does not resolve the issue.

Here I hit f5 and no breakpoint was triggered.

If I comment out the anonymous record, it works:

[cartermp]

Confirmed the full name does not have the ':

FullName = "<>f__AnonymousType1324739542`1[[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]";