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 ConstructorBuilder implementation, integrate TypeBuilder.GetXyz() static methods #94732

Merged
merged 13 commits into from Nov 18, 2023

Conversation

buyaa-n
Copy link
Member

@buyaa-n buyaa-n commented Nov 14, 2023

Constributes to #92975

  • Add ConstructorBuilder implementation
  • Add DefineConstructorCore and DefineDefaultConstructorCore protected members for TypeBuilderImpl
  • Integrate TypeBuilder.GetConstructor(...), TypeBuilder.GetMethod(...) and TypeBuilder.GetField(...) static methods so that they could be used for the new Reflection.Emit.TypeBuilderImpl
  • Fix/implement other APIs that needed for accessing/writing types and methods constructed from runtime type and TypeBuilderImpl type (instances of *BuilderInstantiation types, like MethodBuilderInstantiation)
  • Populate type and its members tokens on CreateTypeInfoCore(). Refactor Assembly.Save IL emit logic to require referenced types to be created ("baked").

@ghost
Copy link

ghost commented Nov 14, 2023

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

Issue Details

Constributes to #92975

  • Add ConstructorBuilder implementation
  • Add DefineConstructorCore and DefineDefaultConstructorCore protected members for TypeBuilderImpl
  • Integrate TypeBuilder.GetConstructor(...), TypeBuilder.GetMethod(...) and TypeBuilder.GetField(...) static methods so that they could be used for the new Reflection.Emit.TypeBuilderImpl
  • Fix/implement other APIs that needed for accessing/writing constructed types from runtime type and TypeBuilderImpl type
Author: buyaa-n
Assignees: -
Labels:

area-System.Reflection.Emit

Milestone: -

Copy link
Member

@steveharter steveharter left a comment

Choose a reason for hiding this comment

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

Some misc feedback but otherwise LGTM.

@@ -286,7 +286,7 @@ public override Type MakeArrayType(int rank)
public override int GetArrayRank()
{
if (!IsArray)
throw new NotSupportedException(SR.NotSupported_SubclassOverride);
throw new ArgumentException(SR.Argument_HasToBeArrayClass);
Copy link
Member

Choose a reason for hiding this comment

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

InvalidOperationException instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Followed exception type and message for similar cases

if (!arrayClass.IsArray)
{
throw new ArgumentException(SR.Argument_HasToBeArrayClass);
}


for (int i = 0; i < _parameterTypes.Length; i++)
{
if (_parameterBuilders[i + 1] == null)
Copy link
Member

Choose a reason for hiding this comment

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

When is the element null in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

When the parameters created with parameterTypes array:

if (parameterTypes != null)
{
_parameterTypes = new Type[parameterTypes.Length];
_parameterBuilders = new ParameterBuilderImpl[parameterTypes.Length + 1]; // parameter 0 reserved for return type
for (int i = 0; i < parameterTypes.Length; i++)
{
ArgumentNullException.ThrowIfNull(_parameterTypes[i] = parameterTypes[i], nameof(parameterTypes));
}

but not explicitly defined with DefineParameterCore(...)

@buyaa-n buyaa-n merged commit f07ae0d into dotnet:main Nov 18, 2023
175 of 177 checks passed
@buyaa-n buyaa-n deleted the ctor_impl branch November 18, 2023 02:25
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants