Skip to content

Disallow types named 'scoped'#62968

Merged
jcouv merged 1 commit intodotnet:release/dev17.4from
jcouv:reserve-scoped
Jul 28, 2022
Merged

Disallow types named 'scoped'#62968
jcouv merged 1 commit intodotnet:release/dev17.4from
jcouv:reserve-scoped

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Jul 27, 2022

@jcouv jcouv self-assigned this Jul 27, 2022
@ghost ghost added the Area-Compilers label Jul 27, 2022
@jcouv jcouv changed the title Move more code over Disallow types named 'scoped' Jul 27, 2022
@alrz
Copy link
Copy Markdown
Member

alrz commented Jul 27, 2022

All lowercase types are already disallowed, no?

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jul 27, 2022

All lowercase types are already disallowed, no?

We only produce a warning wave warning for those. So we escalate to an error when we actually start using a lowercase word as contextual keyword.

@jcouv jcouv marked this pull request as ready for review July 27, 2022 16:01
@jcouv jcouv requested a review from a team as a code owner July 27, 2022 16:01
@@ -478,7 +478,8 @@ internal static void ReportReservedTypeName(string? name, CSharpCompilation comp

if (reportIfContextual(SyntaxKind.RecordKeyword, MessageID.IDS_FeatureRecords, ErrorCode.WRN_RecordNamedDisallowed)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious why records is warning and other contextual keywords are errors?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would have to dig up the PR and/or LDM notes. Most likely this was evolution of our philosophy towards such breaks. Records came before we added the warning wave warning.

@jcouv jcouv merged commit 498db64 into dotnet:release/dev17.4 Jul 28, 2022
@jcouv jcouv deleted the reserve-scoped branch July 28, 2022 05:45
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.

5 participants