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

Disallow default as a constant pattern. #23499

Closed
gafter opened this issue Nov 30, 2017 · 9 comments · Fixed by #23629
Closed

Disallow default as a constant pattern. #23499

gafter opened this issue Nov 30, 2017 · 9 comments · Fixed by #23629
Assignees
Labels
Area-Compilers Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece.
Milestone

Comments

@gafter
Copy link
Member

gafter commented Nov 30, 2017

The LDM has tentatively decided to disallow a default keyword (literal) as a constant pattern, in order to reduce confusion with "default" cases in switch statements and expressions. A programmer can always write null, 0, or '\0' instead. This is technically a breaking change.

In a switch statement, the case label
case default:
is proposed to produce an error something like
error CS8313: A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern '_'.
We already produce approximately this diagnostic, but as a warning.

In an is-pattern expression and other places where patterns appear
e is default
is proposed to produce an error something like
error CS8405: A default literal 'default' is not valid as a pattern. Use another literal (e.g. '0' or 'null') as appropriate. To match everything, use a discard pattern '_'.

This will need review from the compatibility council.

@gafter gafter added Area-Compilers Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. labels Nov 30, 2017
@gafter gafter added this to the 16.0 milestone Nov 30, 2017
@gafter gafter self-assigned this Nov 30, 2017
@jcouv
Copy link
Member

jcouv commented Nov 30, 2017

Just to clarify the proposal is to produce an error instead of the current warning: warning CS8313: Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.

@gafter
Copy link
Member Author

gafter commented Nov 30, 2017

Yes. Clarified above.

@jcouv
Copy link
Member

jcouv commented Nov 30, 2017

Should we make this change in 15.6 or 15.7 instead, to reduce the impact of breaking change? Default expressions where introduced in C# 7.1.

@gafter
Copy link
Member Author

gafter commented Nov 30, 2017

That is a question for the compat council.

@jcouv
Copy link
Member

jcouv commented Nov 30, 2017

I'm not on that thread ;-)
I think we should do this in 15.6. Thanks

@gafter
Copy link
Member Author

gafter commented Dec 7, 2017

For 15.6, the messages are modified as we do not have a discard pattern. For the switch, we recommend the default: label. For pattern matching we recommend var _.

@gafter gafter added the 4 - In Review A fix for the issue is submitted for review. label Dec 7, 2017
gafter added a commit to gafter/roslyn that referenced this issue Dec 7, 2017
Has been reviewed by LDM and compat council.
Fixes dotnet#23499
@gafter gafter added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 4 - In Review A fix for the issue is submitted for review. labels Dec 10, 2017
gafter added a commit that referenced this issue Dec 10, 2017
* Forbid 'default' as a case constant or a pattern.

Fixes #23499
@ericwj
Copy link

ericwj commented Jan 18, 2018

I disagree.

I see no reason why it is necessary to break my code after fixing case default: with case (default): based on the warning previously issued.

CS8313 re-introduces code edits that were happily a thing of the past changing the type of a thing.

And the remark If you intended to write the default label use 'default:' without 'case'. is totally lame. I don't mean default: I mean either case 0: or case null: or ... depending on the actual type.

You won't accidentally write case (default): when you meant default: imho.

karb0f0s added a commit to TelegramBots/Telegram.Bot.Extensions.Exceptions that referenced this issue May 5, 2018
- fix error CS8363 [disallow `default` as a constant pattern](dotnet/roslyn#23499) in tests
@YairHalberstadt
Copy link
Contributor

The suggested solution will of course not work for generic types without a class or a struct constraint

@alrz
Copy link
Member

alrz commented Dec 13, 2018

is it too late to also disallow goto case default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants