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

Change how pre-processor symbols are verified #15900

Closed
jaredpar opened this issue Dec 14, 2016 · 2 comments
Closed

Change how pre-processor symbols are verified #15900

jaredpar opened this issue Dec 14, 2016 · 2 comments
Assignees
Labels
4 - In Review A fix for the issue is submitted for review. Area-Compilers Bug
Milestone

Comments

@jaredpar
Copy link
Member

Today the CSharpParseOptions type is inconsistent with how it verifies pre-processor symbols:

  • The constructor will verify all symbols are valid identifiers and throw an exception when they are not.
  • The WithPreProcessorSymbols method does no verification.

This inconsistency is bad for bot the compiler and consumers of the compiler API. After some discussion the compiler team decided to change the API in the following way:

  • The CSharpParseOptions type will never throw on pre-processor symbols that are invalid identifiers.
  • Diagnostics will be produced for invalid identifiers in the pre-processor symbols

As a part of fixing this we should examine uses like in the following PR to see if they need to be cleaned up after the change.

#15798

@jaredpar jaredpar added this to the 2.0 (RTM) milestone Dec 14, 2016
@jaredpar jaredpar modified the milestones: 2.1, 2.0 (RTM) Jan 12, 2017
@jaredpar
Copy link
Member Author

Doesn't meet the bar now. Will handle in next release.

@OmarTawfik
Copy link
Contributor

OmarTawfik commented Feb 2, 2017

@jaredpar filed #16913 while reading through this.

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 Bug
Projects
None yet
Development

No branches or pull requests

2 participants