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

[release/7.0] Allow underscores in identifiers again #29822

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

AndriySvyryd
Copy link
Member

Port of #29821
Fixes #29450

Description

A change in the validation of identifier names resulted in an unintentional breaking change.

Customer impact

If the generated files would contain underscores the customer needs to add them manually to allow them to compile against existing code.

How found

Customer reported on 7.0

Regression

Yes.

Testing

Adjusted the tests for the affected scenario

Risk

Low; the fix reverts the affected code to the 6.0 version.

@AndriySvyryd AndriySvyryd requested a review from a team December 10, 2022 01:56
@@ -1455,10 +1455,11 @@ private static bool IsIdentifierPartCharacter(char ch)
{
if (ch < 'a')
{
return ch < 'A'
return (ch < 'A'
Copy link
Contributor

Choose a reason for hiding this comment

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

quirk?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's hard to turn on quirks when running tools. Also, it's just reverting to code to what it was before 7.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Fwiw, I have only had two reports on this in Power Tools

@AndriySvyryd AndriySvyryd added this to the 7.0.3 milestone Dec 15, 2022
@wtgodbe wtgodbe merged commit 991290e into release/7.0 Jan 4, 2023
@wtgodbe wtgodbe deleted the Issue29450_7.0 branch January 4, 2023 18:33
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