Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Generate System.Runtime ref assembly against implementation #37737

Merged
merged 2 commits into from
May 17, 2019

Conversation

safern
Copy link
Member

@safern safern commented May 17, 2019

We're about to add Nullable Annotations from Corelib's implementation into System.Runtime reference assembly. In order to make the diff easier when adding those annotations and reducing noise I generated the reference assembly again against the implementation.

Some things that stand out are:

  1. With my change nested ValueTuples now use the (T1, ..., TN) syntax. I.e: Before a nested ValueTuple would look like: (T1, System.ValueTuple<T2, T3>). After my change now they will be emitted as follows: (T1, (T2, T3)).
  2. There were some overrides we weren't exposing in the reference assembly.
  3. One obsolete attribute we were missing on an API marked as Obsolete in Full Framework and in NETStandard.
  4. Some baselined readonly structs.
  5. Cleaned up the baseline as well.
  6. Some references were not using the full type name.
  7. Some ordering differences.

cc: @stephentoub @danmosemsft @ericstj @tannergooding @ahsonkhan @terrajobst

{
public partial class TypeDelegator
{
public override bool IsVariableBoundArray { get { throw null; } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be manual? This looks like a bug in the implementation that this is working around.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this override doesn’t exist in the implementation. I tried to see when it was added but it was added on the initial commit to corefx. The implementation in coreclr has never had it.

Since the implementation doesn’t have it, if you run GenAPI it would remove it, so to avoid that I moved it to Manual.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the implementation doesn’t have it,

Right - this looks like an oversight in the implementation. It would be better to fix the implementation instead of adding a workaround here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll fix the implementation and put this back on the generated ref assembly.

@safern
Copy link
Member Author

safern commented May 17, 2019

Any other concerns before I merge?

@safern safern merged commit 959168e into dotnet:master May 17, 2019
@safern safern deleted the NormalizeSystemRuntime branch May 17, 2019 21:20
@karelz karelz added this to the 3.0 milestone May 22, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…orefx#37737)

* Generate System.Runtime ref assembly against implementation

* PR Feedback and baseline uapaot errors


Commit migrated from dotnet/corefx@959168e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants