Update references assemblies to include private fields for structs by weshaggard · Pull Request #26286 · dotnet/corefx

weshaggard · 2018-01-12T01:32:17Z

Contributes to https://github.com/dotnet/corefx/issues/6185.

This is an update to the rest of the reference assemblies in corefx. I updated every reference assembly that as a struct with any private fields. Some of the refs that weren't easy to regenerate I did targeted fixes for and others I regenerated with the latest tools so there are some other normalization changes in some of the refs.

PTAL @jkotas @VSadov @jaredpar @gafter @tmat @nguerrera

weshaggard · 2018-01-12T01:32:51Z

For reference the tools change is dotnet/buildtools#1859

jkotas · 2018-01-12T02:18:30Z


+  <PropertyGroup Condition="'$(IsReferenceAssembly)' == 'true'">
+    <!-- disable warnings about unused fields -->
+    <NoWarn>$(NoWarn);0169;0649</NoWarn>


Delete this from System.Runtime.csproj now that it is centrally defined?

weshaggard · 2018-01-12T16:32:10Z


 namespace System.ComponentModel.Composition
 {
+    public static partial class AdaptationConstants


@maryamariyan when I regenerated the ref for System.ComponentModel.Composition a few more public API showed up. Can you confirm you didn't intentionally exclude them originally.?

weshaggard · 2018-01-12T16:34:43Z

@@ -82,17 +82,17 @@ public enum MethodSemanticsAttributes
 }
 namespace System.Reflection.Metadata


@tmat @nguerrera could you guys please have a scan over the changes to this ref?

I took a look and it looks fine.

weshaggard · 2018-01-12T16:36:56Z

 // ------------------------------------------------------------------------------

-
 namespace System.Security.Cryptography


@bartonjs would you mind having a scan over the crypto ref's I've regenerated?

Crypto deltas LGTM.

weshaggard · 2018-01-12T16:40:40Z

        public void Start<TStateMachine>(ref TStateMachine stateMachine) where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { }
    }
-    public partial struct ConfiguredValueTaskAwaitable<TResult>
+    public readonly partial struct ConfiguredValueTaskAwaitable<TResult>


@stephentoub while regenerating this a few things got marked as readonly I assume that is more leftover from your other work. In this ref ConfiguredValueTaskAwaiter and ValueTaskAwaiter<TResult> also got marked as readonly based on what the netstandard implementation is but they aren't marked that way in corelib, so I undid it in the ref. Please feel free to make the consistent as you deem fit.

Contributes to https://github.com/dotnet/corefx/issues/6185.

bartonjs · 2018-01-12T17:46:37Z

    public sealed partial class SafeX509ChainHandle : Microsoft.Win32.SafeHandles.SafeHandleZeroOrMinusOneIsInvalid
    {
-        internal SafeX509ChainHandle() : base(default(bool)) { }
-        public override bool IsInvalid { get { throw null; } }


I assume the override was "removed" because the type is sealed (so there's no base.IsInvalid concern)?

it was removed because it wasn't in the implementation.

Ah. That would be a good reason, too 😄.

Looks like when we brought back SafeHandleZeroOrMinusOneIsInvalid the override was removed from the src copy but not the ref.

tmat · 2018-01-12T19:33:45Z

    private readonly object _dummy;

Blob has fields of both reference and value types: byte[] and int. I assume that once there is a _dummy of a reference type we don't need to also add a value type _dummy, but the rules here do not mention that: dotnet/buildtools#1859

Refers to: src/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs:208 in 2e97d43. [](commit_id = 2e97d43, deletion_comment = False)

tmat · 2018-01-12T19:36:48Z

    private readonly TType _Type_k__BackingField;

why not call this _dummy as well (or _dummy0..n) if we need one for each generic paramter.

Refers to: src/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs:497 in 2e97d43. [](commit_id = 2e97d43, deletion_comment = False)

weshaggard · 2018-01-12T19:58:23Z

Blob has fields of both reference and value types: byte[] and int. I assume that once there is a _dummy of a reference type we don't need to also add a value type _dummy, but the rules here do not mention that: dotnet/buildtools#1859

If it already has the reference one we don't need the value type one for the compiler to do it's job.

why not call this _dummy as well (or _dummy0..n) if we need one for each generic paramter.

For the generic fields I just copy them as is because I didn't want to normalize their types.

tmat · 2018-01-12T20:41:27Z

Re the generic types - my only concern is using compiler generated names of the property backing fields (in theory they might change in next compiler version).

weshaggard · 2018-01-12T21:31:06Z

Re the generic types - my only concern is using compiler generated names of the property backing fields (in theory they might change in next compiler version).

I don't expect that the name will be used for anything so I don't think it matters.

…ieldsForStructsInRefs Update references assemblies to include private fields for structs Commit migrated from dotnet/corefx@9d32b61

jkotas approved these changes Jan 12, 2018

View reviewed changes

jkotas reviewed Jan 12, 2018

View reviewed changes

weshaggard commented Jan 12, 2018

View reviewed changes

Update references assemblies to include private fields for structs

2e97d43

Contributes to https://github.com/dotnet/corefx/issues/6185.

weshaggard force-pushed the UpdatePrivateFieldsForStructsInRefs branch from 2bb1b19 to 2e97d43 Compare January 12, 2018 17:08

bartonjs reviewed Jan 12, 2018

View reviewed changes

weshaggard merged commit 9d32b61 into dotnet:master Jan 12, 2018

karelz added this to the 2.1.0 milestone Jan 20, 2018

eerhardt mentioned this pull request Jan 25, 2018

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

Merged

VSadov mentioned this pull request Jan 25, 2018

Blittable design update dotnet/csharplang#1276

Merged

tannergooding mentioned this pull request Jan 27, 2018

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

Closed

gafter mentioned this pull request Jan 31, 2020

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

Closed

		@@ -82,17 +82,17 @@ public enum MethodSemanticsAttributes
		}
		namespace System.Reflection.Metadata

		// ------------------------------------------------------------------------------


		namespace System.Security.Cryptography

Conversation

weshaggard commented Jan 12, 2018

Uh oh!

weshaggard commented Jan 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weshaggard Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmat commented Jan 12, 2018

Uh oh!

tmat commented Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weshaggard commented Jan 12, 2018

Uh oh!

tmat commented Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weshaggard commented Jan 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

weshaggard Jan 12, 2018 •

edited

Loading

tmat commented Jan 12, 2018 •

edited

Loading

tmat commented Jan 12, 2018 •

edited

Loading