Skip to content

Support applying attributes to properties and fields that are created to back primary constructor parameters in records. #45908

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 13, 2020
Merged

Support applying attributes to properties and fields that are created to back primary constructor parameters in records. #45908

merged 1 commit into from
Jul 13, 2020

Conversation

@AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented Jul 11, 2020

Closes #44691.

@AlekseyTs
Support applying attributes to properties and fields that are created…
58233a7 
… to back primary constructor parameters in records.

Closes dotnet#44691.
@AlekseyTs AlekseyTs requested review from a team, RikkiGibson, cston and jcouv July 11, 2020 01:29
@AlekseyTs
Copy link
Contributor Author

AlekseyTs commented Jul 13, 2020 •
edited
Loading

@cston, @jcouv, @RikkiGibson, @dotnet/roslyn-compiler Please review. #Closed

@AlekseyTs
Copy link
Contributor Author

AlekseyTs commented Jul 13, 2020 •
edited
Loading

@cston, @jcouv, @dotnet/roslyn-compiler Please review, need a second sign-off. #Closed

@jcouv
Copy link
Member

jcouv commented Jul 13, 2020 •
edited by AlekseyTs
Loading

Looking #Resolved

jcouv
jcouv reviewed Jul 13, 2020
View reviewed changes
src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs
return attributes.Select(a => a.ToString());
}

internal static IEnumerable<string> GetAttributeStrings(IEnumerable<CSharpAttributeData> attributes)
Copy link
Member

@jcouv jcouv Jul 13, 2020 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

IEnumerable [](start = 64, length = 11)

nit: Can we remove the ImmuableArray overload since this overload will handle that case too? #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs Jul 13, 2020

Choose a reason for hiding this comment

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

Can we remove the ImmuableArray overload since this overload will handle that case too?

I do not think this would be the right thing to do. That overload allocates less.

In reply to: 453887146 [](ancestors = 453887146)

jcouv
jcouv approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 1)

@jcouv jcouv self-assigned this Jul 13, 2020
@AlekseyTs AlekseyTs merged commit ba014d9 into dotnet:master Jul 13, 2020
@ghost ghost added this to the Next milestone Jul 13, 2020
@JoeRobich JoeRobich modified the milestones: Next, 16.8.P1 Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@jcouv jcouv jcouv approved these changes

@cston cston Awaiting requested review from cston

Assignees

@jcouv jcouv

Labels

Area-Compilers Feature - Records Records

Projects

None yet

Milestone

16.8.P1

Development

Successfully merging this pull request may close these issues.

Declare attributes on positional record property

4 participants

@AlekseyTs @jcouv @RikkiGibson @JoeRobich