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

STL: When wchar_t is a real type, users shouldn't see unsigned short machinery #216

Closed
StephanTLavavej opened this issue Oct 25, 2019 · 1 comment · Fixed by #2164
Closed
Labels
enhancement Something can be improved fixed Something works now, yay!

Comments

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 25, 2019

Because the STL grudgingly supports /Zc:wchar_t- (see #212), we occasionally need to detect _NATIVE_WCHAR_T_DEFINED.

For a correct (although weird) example, the STL always defines numeric_limits<wchar_t>:

STL/stl/inc/limits

Lines 460 to 461 in 6b0238d

template <>
class numeric_limits<wchar_t> : public _Num_int_base {

Then if wchar_t is a real type, it additionally defines numeric_limits<unsigned short>:

STL/stl/inc/limits

Lines 685 to 688 in 6b0238d

#ifdef _NATIVE_WCHAR_T_DEFINED
// CLASS numeric_limits<unsigned short>
template <>
class numeric_limits<unsigned short> : public _Num_int_base {

This is backwards, but it works. Users with real wchar_t get both specializations, while users with fake wchar_t get only one.

However, the STL appears to have incorrect logic elsewhere. It follows the numeric_limits pattern for char_traits:

STL/stl/inc/xstring

Lines 265 to 275 in 6b0238d

// STRUCT char_traits<wchar_t>
template <>
struct char_traits<wchar_t> : _WChar_traits<wchar_t> {}; // properties of a string or stream wchar_t element
#ifdef _NATIVE_WCHAR_T_DEFINED
// STRUCT char_traits<unsigned short>
template <>
struct char_traits<unsigned short> : _WChar_traits<unsigned short> {
// properties of a string or stream unsigned short element
};
#endif // _NATIVE_WCHAR_T_DEFINED

But when wchar_t is a real type, char_traits<unsigned short> doesn't make any sense!

<fstream> has many similar examples. basic_ifstream is always constructible from const wchar_t*:

STL/stl/inc/fstream

Lines 823 to 824 in 6b0238d

explicit basic_ifstream(
const wchar_t* _Filename, ios_base::openmode _Mode = ios_base::in, int _Prot = ios_base::_Default_open_prot)

And when wchar_t is real, it's also constructible from const unsigned short*:

STL/stl/inc/fstream

Lines 850 to 852 in 6b0238d

#ifdef _NATIVE_WCHAR_T_DEFINED
explicit basic_ifstream(const unsigned short* _Filename, ios_base::openmode _Mode = ios_base::in,
int _Prot = ios_base::_Default_open_prot)

I believe that all of our _NATIVE_WCHAR_T_DEFINED logic needs to be audited. When we're separately compiling the STL, wchar_t is always a real type, and we additionally need to provide unsigned short counterparts (so we should detect whether we're building the STL). But when headers are being used to compile user code, users with real wchar_t should see only wchar_t for character-ish things, never unsigned short. (Users with fake wchar_t will see only wchar_t being a typedef for unsigned short, as they should.)

In theory, this should not affect ABI compatibility, although it may affect source compatibility (if users with real wchar_t somehow took a dependency on the unsigned short machinery).

Also tracked by Microsoft-internal VSO-387426 / AB#387426.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Oct 25, 2019
@StephanTLavavej
Copy link
Member Author

Although this is guarded by _CRTBLD, ideally it shouldn't appear in the header at all:

STL/stl/inc/iosfwd

Lines 259 to 264 in bd7adb4

#if defined(_CRTBLD)
// unsigned short TYPEDEFS
using ushistream = basic_istream<unsigned short, char_traits<unsigned short>>;
using ushostream = basic_ostream<unsigned short, char_traits<unsigned short>>;
using ushfilebuf = basic_filebuf<unsigned short, char_traits<unsigned short>>;
#endif // defined(_CRTBLD)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant