Skip to content

Report level 7 name warning for record when langversion is below 9#58452

Merged
333fred merged 2 commits intodotnet:mainfrom
333fred:record-warn-lower-langversion
Dec 28, 2021
Merged

Report level 7 name warning for record when langversion is below 9#58452
333fred merged 2 commits intodotnet:mainfrom
333fred:record-warn-lower-langversion

Conversation

@333fred
Copy link
Copy Markdown
Member

@333fred 333fred commented Dec 21, 2021

Another small bug I noticed when working on required members and touching this function. We currently do not report a level 7 warning for 'record' when the langversion is below the minimum for records.

Another small bug I noticed when working on required members and touching this function. We currently do not report a level 7 warning for 'record' when the langversion is below the minimum for records.
@333fred 333fred requested a review from a team as a code owner December 21, 2021 22:35
@ghost ghost added the Area-Compilers label Dec 21, 2021
@333fred
Copy link
Copy Markdown
Member Author

333fred commented Dec 21, 2021

@dotnet/roslyn-compiler for a small review.

@333fred
Copy link
Copy Markdown
Member Author

333fred commented Dec 27, 2021

@dotnet/roslyn-compiler for a small review.

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.

I'd left the record case intentionally alone but that doesn't make much sense. I agree the warning should apply too. LGTM Thanks (iteration 2)

@jcouv jcouv self-assigned this Dec 27, 2021
@333fred
Copy link
Copy Markdown
Member Author

333fred commented Dec 27, 2021

Yeah, my reasoning is that, since the warning is only possibly true for most identifiers, but is definitely true for record, we should warn there.

@dotnet/roslyn-compiler for a second small review.

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 2)

@333fred 333fred merged commit 3028ee7 into dotnet:main Dec 28, 2021
@333fred 333fred deleted the record-warn-lower-langversion branch December 28, 2021 19:36
@ghost ghost added this to the Next milestone Dec 28, 2021
@Cosifne Cosifne modified the milestones: Next, 17.1.P3 Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants