Update CSharpWriter to handle private fields in structs #1859

weshaggard · 2018-01-11T00:20:10Z

For compile-time compat, the following rules should work for producing a reference assembly. We drop all private fields, but add back certain synthesized private fields for a value type (struct) as follows:

If there are any private fields that are or contain any value type members, add a single private field of type int
If there are any private fields that are or contain any reference type members, add a single private field of type object.
If the type is generic, then for every type parameter of the type, if there are any private fields that are or contain any members whose type is that type parameter, we add a direct private field of that type.

For more details see issue https://github.com/dotnet/corefx/issues/6185
this blog is helpful as well http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html

PTAL @jkotas @VSadov @jaredpar @gafter

To get an idea of what is produced see the updated System.Runtime with dotnet/corefx@a2d4a32.

src/Microsoft.Cci.Extensions/Extensions/CSharp/CSharpCciExtensions.cs

+                if (resolvedType is Dummy)
+                    return true;
+
+                foreach (var field in resolvedType.Fields)


jkotas · 2018-01-11T00:44:21Z

Does this also check that the implementation is consistent with the contract; or does this just generate?

weshaggard · 2018-01-11T00:57:24Z

Does this also check that the implementation is consistent with the contract; or does this just generate?

Right now it is just generating. I will see if I can write up an APICompat rule for it but I think that is going to be even more complex to reason about.

I did compare results between generating with coreclr vs uapaot. It generated the same results with the only difference being https://github.com/dotnet/corefx/pull/26264/files#diff-b3d39e42f75b9efe89d21eb385ed61f6R2001 RuntimeArgumentHandle where CoreCLR had a field and UAPAOT didn't. You can see the UAPAOT generated ref here weshaggard/corefx@ca4a3d3 if you are interested.

src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs

+                // - If there are any private fields that are or contain any value type members, add a single private field of type int
+                // - If there are any private fields that are or contain any reference type members, add a single private field of type object.
+                // - If the type is generic, then for every type parameter of the type, if there are any private fields that are or contain any members whose type is that type parameter, we add a direct private field of that type.
+


jkotas · 2018-01-11T01:09:49Z

Right now it is just generating.

Ok, sounds good.

RuntimeArgumentHandle where CoreCLR had a field and UAPAOT didn't

I would expect to have more differences. E.g. RuntimeTypeHandle should have object on CoreCLR and int on CoreRT.

weshaggard · 2018-01-11T17:26:02Z

I would expect to have more differences. E.g. RuntimeTypeHandle should have object on CoreCLR and int on CoreRT.

RuntimeTypeHandle on CoreRT has a EETypePtr which as a pointer field https://github.com/dotnet/corert/blob/master/src/System.Private.CoreLib/src/System/EETypePtr.cs#L27 so it gets flagged as being a reference and ends up with an object dummy field as well.

For compile-time compat, the following rules should work for producing a reference assembly. We drop all private fields, but add back certain synthesized private fields for a value type (struct) as follows: - If there are any private fields that are or contain any value type members, add a single private field of type int - If there are any private fields that are or contain any reference type members, add a single private field of type object. - If the type is generic, then for every type parameter of the type, if there are any private fields that are or contain any members whose type is that type parameter, we add a direct private field of that type. For more details see issue https://github.com/dotnet/corefx/issues/6185 this blog is helpful as well http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html

jkotas · 2018-01-11T17:53:35Z

RuntimeTypeHandle on CoreRT has a EETypePtr which as a pointer

Unmanaged pointers should be treated as int. They should not be treated as object.

IsReferenceType is not always the inverse of IsValueType. Things like unmanaged pointer references aren't value types nor are they a managed reference type. Changed to use the logic already in CCI to detect a reference type but it requires a resolved type definition instead of a type reference.

weshaggard · 2018-01-11T18:27:35Z

@jkotas your right there are a few more differences in UAPAOT implementation. See weshaggard/corefx@71347e7 to see the diff between System.Runtime generated by CoreCLR and then generated by UAPAOT. I think the changes for CoreCLR are the most general and should handle the cases for UAPAOT as well, but let me know if you think there are any differences that are worrisome.

The runtime treats ByReference<T> special as a ref T, so we need the tools to also treat it as a reference.

src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs

+                        if (hasRefPrivateField)
+                            fieldType = parentType.PlatformType.SystemObject;
+
+                        // For primiative types that have a field of their type set the dummy field to that type


src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs

+                        bool hasRefPrivateField = excludedFields.Any(f => f.Type.IsOrContainsReferenceType());
+                        ITypeReference fieldType = parentType.PlatformType.SystemInt32;
+
+                        // If at least one the private fields contains a reference type then we need to


jkotas · 2018-01-12T03:41:11Z

to see the diff between System.Runtime generated by CoreCLR and then generated by UAPAOT

I have fixed differences that were just plain bugs in dotnet/corert#5238

weshaggard mentioned this pull request Jan 11, 2018

Refresh System.Runtime ref from System.Private.CoreLib in Coreclr dotnet/corefx#26264

Merged

VSadov reviewed Jan 11, 2018

View reviewed changes

weshaggard force-pushed the UpdateToolsToHandleStructFields branch from 91562b6 to 04124b3 Compare January 11, 2018 18:16

weshaggard added 3 commits January 11, 2018 16:07

Escape chars from compiler generated backing fields

f34ee45

Use primitive field type for private fields of primitives

7f72f68

Treat ByReference<T> as a reference type

4b80362

The runtime treats ByReference<T> special as a ref T, so we need the tools to also treat it as a reference.

jkotas reviewed Jan 12, 2018

View reviewed changes

src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs Outdated

if (hasRefPrivateField)

fieldType = parentType.PlatformType.SystemObject;

// For primiative types that have a field of their type set the dummy field to that type

This comment was marked as spam.

Sign in to view

jkotas reviewed Jan 12, 2018

View reviewed changes

Fix typos in comments

2518fca

weshaggard mentioned this pull request Jan 12, 2018

Update references assemblies to include private fields for structs dotnet/corefx#26286

Merged

weshaggard merged commit 5299534 into dotnet:master Jan 12, 2018

eerhardt mentioned this pull request Jan 25, 2018

Adding the Vector64<T> type to the S.R.Intrinsics assembly dotnet/corefx#26432

Merged

tannergooding mentioned this pull request Jan 27, 2018

CSC doesn't care if out parameter not assigned dotnet/roslyn#24493

Closed

lambdageek mentioned this pull request Apr 12, 2018

Reference assemblies need to include private struct fields mono/mono#7859

Closed

gafter mentioned this pull request Jan 31, 2020

Reference assembly missing members for KeyValuePair and ReadOnlyMemory dotnet/runtime#24820

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update CSharpWriter to handle private fields in structs #1859

Update CSharpWriter to handle private fields in structs #1859

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 11, 2018

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 11, 2018 •

edited

Loading

weshaggard commented Jan 11, 2018

jkotas commented Jan 11, 2018

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 12, 2018

Update CSharpWriter to handle private fields in structs #1859

Update CSharpWriter to handle private fields in structs #1859

Conversation

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 11, 2018

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 11, 2018 • edited Loading

weshaggard commented Jan 11, 2018

jkotas commented Jan 11, 2018

weshaggard commented Jan 11, 2018

This comment was marked as spam.

This comment was marked as spam.

jkotas commented Jan 12, 2018

jkotas commented Jan 11, 2018 •

edited

Loading