Skip to content

Conversation

@uweigand
Copy link
Contributor

  • Use Constructor, ConstructorArguments, and NamedArguments in internal
    routines to allow them being overridden (like with CoreCLR).

Even after #55726, System.Text.Json.SourceGeneration.Tests are still failing when using a Mono-based dotnet runtime. This new problem is triggered because the source-generation logic provides a class CustomAttributeDataWrapper derived from CustomAttributeData, which overrides the latter class' accessor routines Constructor, ConstructorArguments, and NamedArguments.

This works with CoreCLR. However, with the current Mono implementation, other routines in CustomAttributeData now crash when called on an instance of the derived class, because they now access internal data members that have never been initialized. Fix this by instead calling the accessor routines like in CoreCLR.

* Use Constructor, ConstructorArguments, and NamedArguments in internal
  routines to allow them being overridden (like with CoreCLR).
@uweigand uweigand requested a review from marek-safar as a code owner July 15, 2021 10:57
@ghost
Copy link

ghost commented Jul 15, 2021

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

Comment on lines 153 to 156
public virtual Type AttributeType
{
get { return ctorInfo.DeclaringType!; }
get { return Constructor.DeclaringType!; }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the code to the shared location once it exists

get { return Constructor.DeclaringType!; }
}

public override string ToString()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the code to the shared location once it exists

@marek-safar
Copy link
Contributor

Closing in favour of #55726

@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2021
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.

3 participants