[API Proposal]: Support ref fields in System.Reflection.Metadata.Ecma335.BlobEncoder

Question

[API Proposal]: Support ref fields in System.Reflection.Metadata.Ecma335.BlobEncoder

cston opened this issue 3 years ago · 10 comments

Background and motivation

Provide methods in System.Reflection.Metadata.Ecma335.BlobEncoder to more directly support emitting ref fields, consistent with the corresponding APIs provided for emitting ref parameters and ref locals.

API Proposal

Provide a FieldTypeEncoder type similar to ParameterTypeEncoder and LocalVariableTypeEncoder.

namespace System.Reflection.Metadata.Ecma335
{
    public readonly struct BlobEncoder
    {
        public FieldTypeEncoder FieldTypeEncoder();
    }

    public readonly struct FieldTypeEncoder
    {
        public BlobBuilder Builder { get; }

        public FieldTypeEncoder(BlobBuilder builder);

        public CustomModifiersEncoder CustomModifiers();

        public SignatureTypeEncoder Type(bool isByRef = false);

        public void TypedReference();
    }
}

API Usage

var blobBuilder = new BlobBuilder();
var encoder = new BlobEncoder(blobBuilder);

var fieldEncoder = encoder.FieldTypeEncoder();

// Ref custom modifiers: modopt(object)
var customModifiersEncoder = fieldEncoder.CustomModifiers();
customModifiersEncoder.AddModifier(MetadataTokens.TypeReferenceHandle(2), isOptional: true);

// Type: int&
var typeEncoder = fieldEncoder.Type(isByRef: true);
typeEncoder.PrimitiveType(PrimitiveTypeCode.Int32);

Alternative Designs

The alternative is the existing API which requires emitting SignatureTypeCode.ByReference and creating a CustomModifierEncoder explicitly. The existing API is sufficient, but inconsistent with the support for ref parameters and ref locals.

Using the existing API:

var blobBuilder = new BlobBuilder();
var encoder = new BlobEncoder(blobBuilder);

var typeEncoder = encoder.FieldSignature();

// Ref custom modifiers: modopt(object)
var customModifiersEncoder = new CustomModifiersEncoder(blobBuilder);
customModifiersEncoder.AddModifier(MetadataTokens.TypeReferenceHandle(2), isOptional: true);

// Type: int&
typeEncoder.Builder.WriteByte((byte)SignatureTypeCode.ByReference);
typeEncoder.PrimitiveType(PrimitiveTypeCode.Int32);

Risks

BlobEncoder already includes a FieldSignature method that returns an encoder for a field type, so having two distinct field encoder methods may be confusing.

    public readonly struct BlobEncoder
    {
        public SignatureTypeEncoder FieldSignature();
        public FieldTypeEncoder FieldTypeEncoder();
    }

Answer 1 · 2022-04-21T04:55:56.000Z

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Provide methods in System.Reflection.Metadata.Ecma335.BlobEncoder to more directly support emitting ref fields, consistent with the corresponding APIs provided for emitting ref parameters and ref locals.

API Proposal

Provide a FieldTypeEncoder type similar to ParameterTypeEncoder and LocalVariableTypeEncoder.

namespace System.Reflection.Metadata.Ecma335
{
    public readonly struct BlobEncoder
    {
        public FieldTypeEncoder FieldTypeEncoder();
    }

    public readonly struct FieldTypeEncoder
    {
        public BlobBuilder Builder { get; }

        public FieldTypeEncoder(BlobBuilder builder);

        public CustomModifiersEncoder CustomModifiers();

        public SignatureTypeEncoder Type(bool isByRef = false);
    }
}

API Usage

var blobBuilder = new BlobBuilder();
var encoder = new BlobEncoder(blobBuilder);

var fieldEncoder = encoder.FieldTypeEncoder();

// Ref custom modifiers: modopt(object)
var customModifiersEncoder = fieldEncoder.CustomModifiers();
customModifiersEncoder.AddModifier(MetadataTokens.TypeReferenceHandle(2), isOptional: true);

// Type: int&
var typeEncoder = fieldEncoder.Type(isByRef: true);
typeEncoder.PrimitiveType(PrimitiveTypeCode.Int32);

Alternative Designs

The alternative is the existing API which requires emitting SignatureTypeCode.ByReference and creating a CustomModifierEncoder explicitly. The existing API is sufficient, but inconsistent with the support for ref parameters and ref locals.

Using the existing API:

var blobBuilder = new BlobBuilder();
var encoder = new BlobEncoder(blobBuilder);

// Ref custom modifiers: modopt(object)
var customModifiersEncoder = new CustomModifiersEncoder(blobBuilder);
customModifiersEncoder.AddModifier(MetadataTokens.TypeReferenceHandle(2), isOptional: true);

// Type: int&
var typeEncoder = encoder.FieldSignature();
typeEncoder.Builder.WriteByte((byte)SignatureTypeCode.ByReference);
typeEncoder.PrimitiveType(PrimitiveTypeCode.Int32);

Risks

No response

Author:	cston
Assignees:	-
Labels:	`api-suggestion`, `area-System.Reflection.Metadata`, `untriaged`
Milestone:	-

Answer 2 · 2022-04-21T04:57:57.000Z

cc @jaredpar, @tmat

Answer 3 · 2022-04-21T15:32:58.000Z

@AaronRobinsonMSFT

Answer 4 · 2022-04-21T16:00:40.000Z

@cston Will the following be needed with this new API. It is in the usage example but is also noted in the Alternative design in a manner that implies it wouldn't be needed if this new API was adopted.

// Ref custom modifiers: modopt(object)
var customModifiersEncoder = fieldEncoder.CustomModifiers();
customModifiersEncoder.AddModifier(MetadataTokens.TypeReferenceHandle(2), isOptional: true);

Edit Nevermind. A new instance is being created rather than using the existing one in the fieldEncoder instance.

Answer 5 · 2022-04-21T16:17:42.000Z

A new instance is being created rather than using the existing one in the fieldEncoder instance.

Actually, in both examples an instance of CustomModifiersEncoder is created. The only difference is that the proposed FieldTypeEncoder type includes a CustomModifiers() method that makes it slightly more obvious how to obtain a CustomModifiersEncoder.

The overall difference between use of the existing API and the proposed API is small. It's perhaps just a question of whether the API for emitting fields should be aligned with the existing APIs for locals and parameters.

Answer 6 · 2022-05-05T17:55:16.000Z

Video

Let's call the method BlobEncoder just Field

namespace System.Reflection.Metadata.Ecma335;

public partial readonly struct BlobEncoder
{
    public FieldTypeEncoder Field();
}

public readonly struct FieldTypeEncoder
{
    public BlobBuilder Builder { get; }

    public FieldTypeEncoder(BlobBuilder builder);
    public CustomModifiersEncoder CustomModifiers();
    public SignatureTypeEncoder Type(bool isByRef = false);
    public void TypedReference();
}

Answer 7 · 2022-05-15T20:54:51.000Z

I'm working on this.

Answer 8 · 2022-05-15T21:03:54.000Z

@teo-tsirpanis Please wait for @cston to confirm he doesn't already have an implementation. Work to support ref fields is close to complete and it is possible the work has been done just not posted yet.

Answer 9 · 2022-05-15T21:21:31.000Z

Please wait for @cston to confirm he doesn't already have an implementation.

I have not implemented this. The compiler is currently using the existing API.

Answer 10 · 2022-05-15T21:22:47.000Z

Thanks @cston. @teo-tsirpanis Looking forward to the PR.