Skip to content

Fix locale types - #3324

Merged
Kristine Toliver (ktoliver) merged 2 commits into
MicrosoftDocs:masterfrom
adr26:locale
Aug 26, 2021
Merged

Kristine Toliver (ktoliver) merged 2 commits into
MicrosoftDocs:masterfrom
adr26:locale

Conversation

@adr26

Copy link
Copy Markdown
Contributor

Fix references to locale_t to use _locale_t instead.

@PRMerger20

Copy link
Copy Markdown
Contributor

Andrew Rogers (@adr26) : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@TylerMSFT

Copy link
Copy Markdown
Collaborator

Andrew Rogers (@adr26), can you tell me a little bit about the motivation for this change?

@adr26

Copy link
Copy Markdown
Contributor Author

Andrew Rogers (@adr26), can you tell me a little bit about the motivation for this change?

Sure Tyler Whitney (@TylerMSFT): I was constructing some test code for CRT printf APIs, after I spotted some errors in the documentation of their behaviour (I'll open another PR for this).

I did a search throughout the CRT code for locale_t, the only hit was in strsafe.h — and then the hit was only in comments on the function signatures, rather than the types of the arguments themselves!

Everywhere else, _locale_t is the type used in the APIs (it being a typedef of __crt_locale_pointers in corecrt.h), and there is no typedef for locale_t.

As such, all the references to locale_t in these docs are incorrect and need to be fixed to use _locale_t instead. Searching for a locale_t typedef in the CRT sources and verifying it doesn't exist may be the easiest way to confirm this assertion is correct, and similarly you can consider that:

#include <locale.h>

int main()
{
    locale_t loc = _get_current_locale();
    return 0;
}

fails to compile:

C:\Temp>cl locale.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.30.30423 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

locale.c
locale.c(5): error C2065: 'locale_t': undeclared identifier
locale.c(5): error C2146: syntax error: missing ';' before identifier 'loc'
locale.c(5): error C2065: 'loc': undeclared identifier
locale.c(5): warning C4047: '=': 'int' differs in levels of indirection from '_locale_t'

whereas:

#include <locale.h>

int main()
{
    _locale_t loc = _get_current_locale();
    return 0;
}

compiles just fine!

C:\Temp>cl locale.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.30.30423 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

locale.c
Microsoft (R) Incremental Linker Version 14.30.30423.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:locale.exe
locale.obj

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 _atodbl_l in docs/c-runtime-library/reference/atodbl-atodbl-l-atoldbl-atoldbl-l-atoflt-atoflt-l.md, updating the text to read:

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 stdlib.h), you see that the sources use _locale_t, and not locale_t:

_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

@colin-home

Copy link
Copy Markdown
Contributor

Tyler Whitney (@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 locale_t in headers since at least 2008, the oldest set of headers on my machine.

@TylerMSFT

Copy link
Copy Markdown
Collaborator

Andrew Rogers (@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."

@TylerMSFT Tyler Whitney (TylerMSFT) left a comment

Copy link
Copy Markdown
Collaborator

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!

@ktoliver
Kristine Toliver (ktoliver) merged commit e5d743a into MicrosoftDocs:master Aug 26, 2021
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.

7 participants