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

Remove if-def from JSON ref file previously used to support linker attributes #41484

Merged
merged 2 commits into from
Aug 28, 2020

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Aug 27, 2020

Addresses #40999. To close that issue, this PR needs to be ported to .NET 5.

Internal versions of the attributes are used instead. They've already been included in the ref project (done in #40185). System.Text.Json is the only assembly with this issue.

  • First commit contains core change.
  • Second commit is the result of auto-generating the ref file with /t:GenerateReferenceSource.

@layomia layomia added this to the 5.0.0 milestone Aug 27, 2020
@layomia layomia self-assigned this Aug 27, 2020
#if NETCOREAPP && !NETCOREAPP3_0
private const System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes MembersAccessedOnRead = System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties;
private const System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes MembersAccessedOnWrite = System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties;
private const System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes MembersAccessedOnRead = System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicProperties | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicFields;
Copy link
Member

Choose a reason for hiding this comment

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

Should this rather use the autogenerated format? (ie what you get when running /t:GenerateReferenceSource)

Copy link
Contributor Author

@layomia layomia Aug 27, 2020

Choose a reason for hiding this comment

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

It probably should. I ran /t:GenerateReferenceSource on the ref project in a follow up commit.

@layomia
Copy link
Contributor Author

layomia commented Aug 27, 2020

FYI @eerhardt @joperezr @safern

@joperezr
Copy link
Member

Fixes #40999.

I'm sure you will, but remember that this will close that issue and we still want this to be cherry-picked into release branch so you'll have to reopen after merging to track that cherry-pick.

@layomia
Copy link
Contributor Author

layomia commented Aug 27, 2020

I'm sure you will, but remember that this will close that issue and we still want this to be cherry-picked into release branch so you'll have to reopen after merging to track that cherry-pick.

Thanks 😃 I'll save myself the extra hop and say that this PR "addresses" that issue.

@layomia layomia merged commit bcfd90d into dotnet:master Aug 28, 2020
@layomia
Copy link
Contributor Author

layomia commented Aug 28, 2020

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/227939769

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.

4 participants