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

[API Proposal]: System.Runtime.CompilerServices.MetadataUpdateOriginalTypeAttribute #66222

Closed
lambdageek opened this issue Mar 4, 2022 · 6 comments · Fixed by #69751
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Diagnostics-coreclr blocking Marks issues that we want to fast track in order to unblock other important work enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented Mar 4, 2022

Background and motivation

In .NET 6 we added CreateNewOnMetadataUpdateAttribute to allow frameworks to mark types that do not carry any state that needs to be preserved between hot reload updates (or that any relevant state can be migrated by the framework itself from old instances of old types to new instances of new types). When Roslyn generates a delta for a type with this attribute, more kinds of edits are allowed, and a new class is emitted with a mangled name, instead of a modification to the metadata of the original class.

Additionally the MetadataUpdateHandlerAttribute allows frameworks to register callbacks that are invoked by the hot reload agent after an update is applied. The handlers receive a collection of the modified types. In the case of CreateNewOnMetadataUpdate types, it is the new types that are included in the collection.

The question arises, how should a framework relate the new types to the old types? or How do frameworks retrieve the original name of the type given only the (mangled) name of the new type that Roslyn generated in the delta?

In ASP.NET Core, .cshtml pages are tagged with [CreateNewOnMetadataUpdate].
In other frameworks, such as Comet, every UI component including user code inherits from a base class that is marked with [CreateNewOnMetadataUpdate], and there is no convenient additional attribute to identify the previous and current versions of a type.

This API proposal introduces a new attribute that Roslyn will emit on updated [CreateNewOnMetadataUpdate] types that will include their original name.

API Proposal

namespace System.Runtime.CompilerServices
    /// Types are marked with this attribute by C# Hot Reload to indicate the original name of a type that has been created by applying hot reload changes in a running application. It is not meant to be used directly by application developers.
    [AttributeUsage(System.AttributeTargets.Class, AllowMultiple=false)]
    public class MetadataUpdateOriginalNameAttribute : Attribute
    {
        /// The fullName parameter is the fully qualified name of the original type, including its namespace but not its assembly.  Same as the Type.FullName property of the original type.
        public MetadataUpdateOriginalNameAttribute(string fullName);
    }
}

API Usage

Given this pair of classes class

[CreateNewOnMetadataUpdate]
public class BaseClass {}

namespace ABC {
  public class DerivedClass : BaseClass {}
}

A subsequent edit

public class DerivedClass : BaseClass {
  public int X {get; set;}
}

will cause roslyn to emit a hot reload delta containing:

namespace ABC {
  [MetadataUpdateOriginalName("ABC.DerivedClass")]
  public class DerivedClass#1 : BaseClass {
    public int X {get; set;}
  }
}

A second edit

public class DerivedClass : BaseClass {
  public int X {get; set;}
  public int Y {get; set;}
}

will cause roslyn to emit a second hot reload delta containing:

namespace ABC {
  [MetadataUpdateOriginalName("ABC.DerivedClass")]
  public class DerivedClass#2 : BaseClass {
  public int X {get; set;}
  public int Y {get; set;}
  }
}

Alternative Designs

  • We could formalize the name mangling that Roslyn uses for the new types and expect framework authors to know how to parse them. (Or provide some API to do the de-mangling).

  • We could employ some metadata encoding scheme for the new types: Roslyn could emit the new types as nested types of the original type, for example. Then to find the relationship between the old and new type we would merely get the enclosing type.

  • A minor variation on the proposal above: Instead of each type carrying an attribute with its original name, instead the attribute could contain the name of the preceding most-recent type (e.g. MetadataUpdateOriginalName("ABC.DerivedClass#1") on DerivedClass#2). This seems like a minor difference. Frameworks likely need to keep track of all previous edits in any case.

  • We could go more general: extend the CompilerGeneratred attribute with an OriginalName property as proposed in
    Support retrieving original type name from mangled type name roslyn#55651 in order to provide a mechanism to reconstruct the original name of other compiler-generated constructs such as lambdas and nested function in addition to reloadable types.

Risks

No response

@lambdageek lambdageek added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 4, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 4, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@lambdageek lambdageek self-assigned this Mar 4, 2022
@lambdageek lambdageek changed the title [API Proposal]: System.Runtime.CompilerServices.MetadataUpdatePreviousTypeAttribute [API Proposal]: System.Runtime.CompilerServices.MetadataUpdateOriginalNameAttribute Mar 5, 2022
@lambdageek
Copy link
Member Author

@captainsafia FYI, since it's treading the same waters as dotnet/roslyn#55651 But that proposal is about the "original name" of other constructs that Roslyn mangles even in the baseline version of an assembly. Whereas this is about names that are mangled specifically as part of C# Hot Reload on classes that compile in an unsurprising way in the baseline assembly.

@captainsafia
Copy link
Member

Quite similar indeed! It would be great if we can get a two-for-one solution with this one but I recognize that it might be easier to solve the hot-reload specific problem.

@tommcdon tommcdon added area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner area-EnC-coreclr labels Mar 7, 2022
@ghost
Copy link

ghost commented Mar 7, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

In .NET 6 we added CreateNewOnMetadataUpdateAttribute to allow frameworks to mark types that do not carry any state that needs to be preserved between hot reload updates (or that any relevant state can be migrated by the framework itself from old instances of old types to new instances of new types). When Roslyn generates a delta for a type with this attribute, more kinds of edits are allowed, and a new class is emitted with a mangled name, instead of a modification to the metadata of the original class.

Additionally the MetadataUpdateHandlerAttribute allows frameworks to register callbacks that are invoked by the hot reload agent after an update is applied. The handlers receive a collection of the modified types. In the case of CreateNewOnMetadataUpdate types, it is the new types that are included in the collection.

The question arises, how should a framework relate the new types to the old types? or How do frameworks retrieve the original name of the type given only the (mangled) name of the new type that Roslyn generated in the delta?

In ASP.NET Core, Razor pages are tagged with [CreateNewOnMetadataUpdate] as well as a [Route] attribute that include the page path in a property. The old and the new types are related if they have the same Route.

In other frameworks, such as Comet (an MVU framework atop .NET MAUI) every UI component including user code inherits from a base class that is marked with [CreateNewOnMetadataUpdate], but there is no convenient additional attribute to identify the previous and current versions of a type.

This API proposal introduces a new attribute that Roslyn will emit on updated [CreateNewOnMetadataUpdate] types that will include their original name.

API Proposal

namespace System.Runtime.CompilerServices
    /// Types are marked with this attribute by C# Hot Reload to indicate the original name of a type that has been created by applying hot reload changes in a running application. It is not meant to be used directly by application developers.
    [AttributeUsage(System.AttributeTargets.Class, AllowMultiple=false)]
    public class MetadataUpdateOriginalNameAttribute : Attribute
    {
        /// The fullName parameter is the fully qualified name of the original type, including its namespace but not its assembly.  Same as the Type.FullName property of the original type.
        public MetadataUpdateOriginalNameAttribute(string fullName);
    }
}

API Usage

Given this pair of classes class

[CreateNewOnMetadataUpdate]
public class BaseClass {}

namespace ABC {
  public class DerivedClass : BaseClass {}
}

A subsequent edit

public class DerivedClass : BaseClass {
  public int X {get; set;}
}

will cause roslyn to emit a hot reload delta containing:

namespace ABC {
  [MetadataUpdateOriginalName("ABC.DerivedClass")]
  public class DerivedClass#1 : BaseClass {
    public int X {get; set;}
  }
}

A second edit

public class DerivedClass : BaseClass {
  public int X {get; set;}
  public int Y {get; set;}
}

will cause roslyn to emit a second hot reload delta containing:

namespace ABC {
  [MetadataUpdateOriginalName("ABC.DerivedClass")]
  public class DerivedClass#2 : BaseClass {
  public int X {get; set;}
  public int Y {get; set;}
  }
}

Alternative Designs

  • We could formalize the name mangling that Roslyn uses for the new types and expect framework authors to know how to parse them. (Or provide some API to do the de-mangling).

  • 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.

  • A minor variation on the proposal above: Instead of each type carrying an attribute with its original name, instead the attribute could contain the name of the preceding most-recent type (e.g. MetadataUpdateOriginalName("ABC.DerivedClass#1") on DerivedClass#2). This seems like a minor difference. Frameworks likely need to keep track of all previous edits in any case.

Risks

No response

Author: lambdageek
Assignees: lambdageek
Labels:

enhancement, api-suggestion, area-Diagnostics-coreclr

Milestone: -

@tommcdon tommcdon added this to the 7.0.0 milestone Mar 7, 2022
@lambdageek lambdageek added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 8, 2022
@tommcdon tommcdon modified the milestones: 7.0.0, Future Apr 27, 2022
@lambdageek lambdageek modified the milestones: Future, 7.0.0 May 16, 2022
@lambdageek lambdageek added the blocking Marks issues that we want to fast track in order to unblock other important work label May 16, 2022
@terrajobst
Copy link
Member

terrajobst commented May 24, 2022

Video

  • Should we add other types (interface, delegate?)
  • We should add a property for consistency
  • We should use System.Type instead of System.String
    • This might be desirable if we reference previous versions or only the root. We believe we'd only reference the root.
    • This also gets rid of all the ambiguities on the actual string format
    • Should be renamed to MetadataUpdateOriginalTypeAttribute then
namespace System.Runtime.CompilerServices;

[AttributeUsage(System.AttributeTargets.Class | System.AttributeTargets.Struct,
                AllowMultiple=false, Inherited=false)]
public class MetadataUpdateOriginalTypeAttribute : Attribute
{
    public MetadataUpdateOriginalTypeAttribute(Type originalType);
    public Type OriginalType { get; }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 24, 2022
@lambdageek lambdageek changed the title [API Proposal]: System.Runtime.CompilerServices.MetadataUpdateOriginalNameAttribute [API Proposal]: System.Runtime.CompilerServices.MetadataUpdateOriginalTypeAttribute May 24, 2022
@lambdageek
Copy link
Member Author

@Clancey any difference between pointing to the root of the edits (ie the original type the user wrote) vs the previous version? what happens in Comet if an instance of a previous version is created after an update has been applied (ie: MyUIComponent#3 is instantiated by some code when MyUIComponent#5 is the latest version that has been sent by a hot reload update). I assume you keep track of all the previous versions and upgrade them to the latest?

@tmat : Is it ok for roslyn to emit a [MetadataUpdateOriginalName(typeof(MyUIComponent))]? (ie: System.Type instead of a string?). Is there a limitation on what kinds of things can be tagged with [CreateNewOnMetadataUpdate] in Roslyn? is it only classes, or struct, enums, interfaces, delegates, too?

lambdageek added a commit to lambdageek/runtime that referenced this issue May 24, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 24, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 31, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Diagnostics-coreclr blocking Marks issues that we want to fast track in order to unblock other important work enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants