-
Notifications
You must be signed in to change notification settings - Fork 206
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
Use /execution-charset:us-ascii only with /WX #2368
base: main
Are you sure you want to change the base?
Use /execution-charset:us-ascii only with /WX #2368
Conversation
Use the MSVC /source-charset:utf-8 /execution-charset:us-ascii options only when warnings are treated as errors (/WX) so that we never substitute a question mark (?) for a non-ASCII character in string or character literals. Update the comments for /source-charset:utf-8 /execution-charset:us-ascii basec on improved understranding of these options. Related to AOMediaCodec#2345.
f5a72ba
to
3f72451
Compare
@tongyuantongyu Yuan: Please do a preliminary review of this pull request. Because of the substitution of a question mark (?) for a non-ASCII character in string or character literals, my first thought is that these options should only be used when warnings are treated as errors. So that's what this pull request does. You also made a good point that for compatibility with clang-cl and with non-Windows platforms, we should use the |
I prefer to always add My intention to add Your new comments explained many possible misunderstandings a user or new maintainer may have. Can you move the explanations to a new paragraph, so the first paragraph explains what we want to do and what the flags do, and the second paragraph explains more details (like "check the strings you added" when seeing C4566) and resolves misunderstandings (like it won't interfere escape sequences and won't prevent printing user provided UTF-8 strings)? |
Yuan: Thank you very much for your comment. I will try to revise the comments based on your suggestions. However, I am now wondering if we should just switch to To fully understand these options, I had to read three pages of Microsoft documentation (for I think it is the I will look into why the vcpkg libavif package removed the |
Use the MSVC /source-charset:utf-8 /execution-charset:us-ascii options only when warnings are treated as errors (/WX) so that we never substitute a question mark (?) for a non-ASCII character in string or character literals.
Update the comments for /source-charset:utf-8
/execution-charset:us-ascii basec on improved understranding of these options.
Related to #2345.