-
Notifications
You must be signed in to change notification settings - Fork 972
Fix locale types #3324
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
Fix locale types #3324
Conversation
@adr26 : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
@adr26, can you tell me a little bit about the motivation for this change? |
Sure @TylerMSFT: I was constructing some test code for CRT I did a search throughout the CRT code for Everywhere else, As such, all the references to #include <locale.h>
int main()
{
locale_t loc = _get_current_locale();
return 0;
} fails to compile:
whereas: #include <locale.h>
int main()
{
_locale_t loc = _get_current_locale();
return 0;
} compiles just fine!
However, if you need more evidence, you may also individually check each change I've made. For example, the first change is to the signature of int _atodbl_l ( _CRT_DOUBLE * value, char * str, _locale_t locale ); instead of: int _atodbl_l ( _CRT_DOUBLE * value, char * str, locale_t locale ); If you compare this with the actual declaration from the CRT sources (in this instance, in _Check_return_
_ACRTIMP int __cdecl _atodbl_l(
_Out_ _CRT_DOUBLE* _Result,
_In_z_ char* _String,
_In_opt_ _locale_t _Locale
); You may likewise repeat this exercise manually for the other 109 instances, over 50 files, where I have fixed this: should you so desire... (though hopefully I've convinced you that this is correct without you having to check each one!) Thanks, Andrew R |
@TylerMSFT I forgot to hit the comment button after writing a note earlier: I checked it out and it's legit. I couldn't tell when that type's name was changed, or if the leading underscores got stripped at some point in our many doc system migrations. I suspect the name change in the Microsoft library dates back to when the POSIX renaming happened twenty years ago. Other libraries support it without the leading underscore, either as an alias or a typedef. We haven't used |
@adr26 , I just finished checking this out as well and your change is approved :-) Thank you very much for doing that pass! That's incredibly helpful. Here's a little bonus material based on a conversation I just had with a dev here about this: "pretty much everything that is POSIX or MSVC-specific we prepend an underscore in front of the name. The reason is because to be standards-compliant with C, we can’t include any non-standard symbols in the standard headers, unless they use a reserved name (any name starting with an underscore). For example, if we didn’t do this and you declared a global variable named “open” or “y0”, you would fail to compile, but standard-conforming C should be allowed to do that. Nowadays, we do still declare these unless STDC is 1 (which it never is because it’s tied to /Za) or _CRT_DECLARE_NONSTDC_NAMES is defined. I believe we put an underscore in front of locale_t for the same reason – it’s not a part of the C standard. For the function names, we also provide oldnames.lib to allow omitting the underscore, but I don’t believe we have anything like this for _locale_t. I imagine the reason is like you said – wasn’t accepted into POSIX until 2008 and at that point it wasn't a priority to use ‘locale_t’ on Windows. Additionally, any C++ users might have a bad time with function overloading if we change type names based on macros like that." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are awesome for doing this sweeping update. Thank you!
Fix references to
locale_t
to use_locale_t
instead.