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

Design the /warnversion command-line interface and compiler API #45941

Closed
gafter opened this issue Jul 13, 2020 · 5 comments
Closed

Design the /warnversion command-line interface and compiler API #45941

gafter opened this issue Jul 13, 2020 · 5 comments
Assignees
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Need Design Review The end user experience design needs to be reviewed and approved. New Feature - Warning Waves Warning Waves
Milestone

Comments

@gafter
Copy link
Member

gafter commented Jul 13, 2020

What are the set of values accepted on the /warnversion compiler command-line flag?

Does it affect/change the /warn setting? Should they be the same (existing) flag?

How can the user discover the set of legal values? How can the user discover the set of warnings enabled by a given version?

@gafter gafter added Concept-API This issue involves adding, removing, clarification, or modification of an API. Area-Compilers New Feature - Warning Waves Warning Waves labels Jul 13, 2020
@gafter gafter added this to the 16.8 milestone Jul 13, 2020
@gafter gafter self-assigned this Jul 13, 2020
@gafter
Copy link
Member Author

gafter commented Jul 13, 2020

/cc @jcouv @333fred @cston @jmarolf

@gafter
Copy link
Member Author

gafter commented Jul 13, 2020

See also #45906

@gafter gafter added the Need Design Review The end user experience design needs to be reviewed and approved. label Jul 13, 2020
@CyrusNajmabadi
Copy link
Member

Note: i liked fred's point about this being a diagnosticwave rather than a warningwave. so maybe it's diagnosticversion. and later diagnosticversions can bring in new diagnostics (which might be warnings or errors). You can then independently change warnings to errors as you see fit.

@gafter gafter added the 4 - In Review A fix for the issue is submitted for review. label Jul 15, 2020
@agocke
Copy link
Member

agocke commented Jul 17, 2020

Copying over my previous vociferous objection to representing version numbers using number types:

Version numbers are not numbers. Thinking of a version number, like 5.1, as a number is a failure of pattern matching. Using a decimal allows you to specify versions that don't exist, like 3.1415926535, and does not let you specify versions that may exist, like 5.1.1.

LangVersion uses an enum, because the correct way to model versions is that the actual supported versions is a closed enumerated set. The actual version number passed on the command line is a string, and could only be accurately represented by a string. Version numbers should be parsed like any other option, then processed into a type-safe format (enum).

@gafter
Copy link
Member Author

gafter commented Sep 3, 2020

We have dropped the proposed /warnversion flag. We are simply adding new values to the existing /warn flag. The first "new" value will be "5" in C# 9. However, the /warn flag permits passing higher values now if you want to accept future warning "waves".

@gafter gafter closed this as completed Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Need Design Review The end user experience design needs to be reviewed and approved. New Feature - Warning Waves Warning Waves
Projects
None yet
Development

No branches or pull requests

3 participants