Skip to content

Filter RequiresLocationAttribute from ref readonly parameters#68870

Merged
jjonescz merged 1 commit intodotnet:features/RefReadonlyfrom
jjonescz:RefReadonly-08-FilterAttribute
Jul 7, 2023
Merged

Filter RequiresLocationAttribute from ref readonly parameters#68870
jjonescz merged 1 commit intodotnet:features/RefReadonlyfrom
jjonescz:RefReadonly-08-FilterAttribute

Conversation

@jjonescz
Copy link
Copy Markdown
Member

@jjonescz jjonescz commented Jul 4, 2023 •
edited
Loading

Seems that PEParameterSymbol.GetAttributes() excludes IsReadOnlyAttribute for in parameters, so analogously, this PR makes ref readonly parameters exclude RequiresLocationAttribute.

Speclet: https://github.com/dotnet/csharplang/blob/cd336064a26a43c31c1164ef7cd3f5feb4420d20/proposals/ref-readonly-parameters.md
Test plan: #68056

@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 4, 2023
@jjonescz jjonescz added Feature - Ref Readonly Parameters `ref readonly` parameters Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead and removed Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 4, 2023
jjonescz
jjonescz commented Jul 4, 2023
View reviewed changes
src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs
Assert.Empty(attribute.ConstructorArguments);
Assert.Empty(attribute.NamedArguments);
}
Assert.Empty(parameter.GetAttributes());
Copy link
Copy Markdown
Member Author

@jjonescz jjonescz Jul 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, it means the presence of the attribute cannot be verified using compiler APIs anymore, I think. But it's still verified in the sense that it makes PEParameterSymbol.RefKind to be decoded correctly as RefReadonlyParameter if the attribute is present.

@jjonescz jjonescz marked this pull request as ready for review July 4, 2023 10:16
@jjonescz jjonescz requested a review from a team as a code owner July 4, 2023 10:16
@jjonescz jjonescz requested review from AlekseyTs and jcouv July 4, 2023 10:16
@jjonescz jjonescz added this to the C# 12.0 milestone Jul 4, 2023
AlekseyTs
AlekseyTs approved these changes Jul 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 1)

@jcouv jcouv self-assigned this Jul 7, 2023
jcouv
jcouv approved these changes Jul 7, 2023
View reviewed changes
Copy link
Copy Markdown
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)

@jjonescz jjonescz merged commit e3ed9a4 into dotnet:features/RefReadonly Jul 7, 2023
@jjonescz jjonescz deleted the RefReadonly-08-FilterAttribute branch July 7, 2023 08:45
@allisonchou allisonchou mentioned this pull request Aug 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlekseyTs AlekseyTs AlekseyTs approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@jcouv jcouv

Labels

Area-Compilers Feature - Ref Readonly Parameters `ref readonly` parameters untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Milestone

C# 12.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jjonescz @AlekseyTs @jcouv