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

Add an attribute specifying the original type, when we create a diff of a ReloadableType #59954

Closed
Clancey opened this issue Mar 4, 2022 · 8 comments · Fixed by #63169
Closed
Assignees
Labels
Area-IDE Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request Interactive-EnC
Milestone

Comments

@Clancey
Copy link

Clancey commented Mar 4, 2022

Background and Motivation

C# Hot Reload added a new CreateNewOnMetadataUpdateAttribute attribute, which creates a new type without any limitations. This will be heavily used in Comet. When the new types are sent to the Framework, we need a way to identify the original type it is replacing.

Proposed API

Let's add an attribute to the new generated types that will specify the original type it is replacing. This will be automatically applied during the diff creation.

[OriginalType(string fullName)]

Now at runtime, a library framework can check which type is being replaced, by querying the attribute.

@Clancey Clancey added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Mar 4, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 4, 2022
@lambdageek
Copy link
Member

lambdageek commented Mar 4, 2022

Should it be the original type, or the previous type? if I make two successive edits, should it go

  • [OriginalType("MyClass")] class MyClass#1 {...} and [OriginalType("MyClass")] class MyClass#2 {...} or
  • [OriginalType("MyClass")] class MyClass#1 {...} and [OriginalType("MyClass#1")] class MyClass#2 {...}

Other comments:

  • fullName seems sufficient. You can only replace a type in the same assembly.

  • This works for nested types, too

@tmat tmat self-assigned this Mar 4, 2022
@Clancey
Copy link
Author

Clancey commented Mar 4, 2022

  • [OriginalType("MyClass")] class MyClass#1 {...} and [OriginalType("MyClass")] class MyClass#2 {...}

It should all reference back to the original type.

@tmat
Copy link
Member

tmat commented Mar 4, 2022

Should it be the original type, or the previous type?

If we call the attribute OriginalName (as in unmangled name) it won't matter.

@tmat
Copy link
Member

tmat commented Mar 4, 2022

Related: #55651

@lambdageek
Copy link
Member

lambdageek commented Mar 4, 2022

Runtime API proposal dotnet/runtime#66222

Edit: I like OriginalName. Updated the API proposal to be S.R.CompilerServices.MetadataUpdateOriginalNameAttribute

@lambdageek
Copy link
Member

@tmat I had a wacky idea while I was working on the API proposal:

Roslyn could emit the new types as nested types of the original type. Then to find the relationship between the old and new type we would merely get the enclosing type.

This is pretty terrible and we don't want it, right?

@tmat
Copy link
Member

tmat commented Mar 5, 2022

Roslyn could emit the new types as nested types of the original type.

Seems a bit obscure and too complicated.

@jinujoseph jinujoseph removed the untriaged Issues and PRs which have not yet been triaged by a lead label Mar 9, 2022
@jinujoseph jinujoseph added this to the 17.3 milestone Mar 9, 2022
@lambdageek
Copy link
Member

lambdageek commented May 25, 2022

The API proposal was approved. Runtime PR is in dotnet/runtime#69751 I would expect it to be in net7 preview 6

namespace System.Runtime.CompilerServices;

[AttributeUsage(System.AttributeTargets.Class | System.AttributeTargets.Struct,
                AllowMultiple=false, Inherited=false)]
public class MetadataUpdateOriginalTypeAttribute : Attribute
{
    /// <summary> This attribute is emited by Roslyn when a type that is marked with (or derives
    /// from a type that is marked with) <see
    /// cref="System.Runtime.CompilerServices.CreateNewOnMetadataUpdateAttribute" /> is updated
    /// during a hot reload session.  The <see cref="OriginalType" /> points to the original version
    /// of the updated type.  The next update of the type will have the same <see
    /// cref="OriginalType" />. Frameworks that provide support for hot reload by implementing a
    /// <see cref="System.Reflection.Metadata.MetadataUpdateHandlerAttribute" /> may use this
    /// attribute to relate an updated type to its original version.  </summary>
    ///
    /// <param name="originalType">The original type that was updated</param>
    public MetadataUpdateOriginalTypeAttribute(Type originalType) => OriginalType = originalType;
    /// Gets the original version of the type that this attribtue is attached to.
    public Type OriginalType { get; }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request Interactive-EnC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants