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

cross-plat: wrap wchar type and string literal #456

Closed
jianchun opened this issue Mar 4, 2016 · 23 comments
Closed

cross-plat: wrap wchar type and string literal #456

jianchun opened this issue Mar 4, 2016 · 23 comments

Comments

@jianchun
Copy link

jianchun commented Mar 4, 2016

wchar_t and L"..." use 2-byte chars on Windows but 4-byte chars on Unix. We need a common representation for cross-platform. We have been experimenting with wchar16 and CH_WSTR in "linux" branch, and realized that's a lot of code noise and would probably result in merge hell. We would like to do a batch replacement in "master" with better named typedef/macro.

C++11 has char16_t and u"...". But unfortunately that's not supported by VS2013.

[UPDATE 3/7]: We are planning with char16 and _u("..."). Below is older message.

We are thinking of using char16 and _U("...").

  • I've noticed char16 already exists on Windows headers. Not sure if that's a blocker. If it doesn't work maybe we need to go back to wchar16.
  • Some suggested to change _U("...") to _u("...") as that looks more closely to standard u"..." notation.

Please chime in if you have ideas. I'm planning to try batch replacements soon.

@jianchun
Copy link
Author

jianchun commented Mar 4, 2016

@ianwjhalliday @tcare @suwc

@tcare
Copy link
Contributor

tcare commented Mar 4, 2016

Have you tried building in SD with these changes to see if char16 works?

@ianwjhalliday
Copy link
Collaborator

FYI @LouisLaf discovered that C++11 requires that function static variable initialization must be thread safe this morning and really liked that benefit. He was starting to agree that it might be really beneficial to min support C++11 compliant compilers.

I don't want to block this work nor stray offtopic, but do want to note that if we did jump to C++11 only compilers then we would be able to use char16_t and u"" directly instead of this typedef and macro.

@digitalinfinity
Copy link
Contributor

@ianwjhalliday that's a great point but I think that's a longer separate discussion that we should have (feel free to open a separate issue). We need to close on this particular issue more rapidly since it's currently impeding progress on the cross-plat efforts.

@ianwjhalliday
Copy link
Collaborator

@digitalinfinity agreed. I'll write up a new issue to discuss C++11.

I like char16 and _u

@dilijev
Copy link
Contributor

dilijev commented Mar 4, 2016

+1 for char16 and _u. They should be easy to find-and-replace if we switch to the C++11 compliant compiler requirement.

@dilijev
Copy link
Contributor

dilijev commented Mar 4, 2016

(Or just not replace since the macros will handle the job in any case.)

@jianchun
Copy link
Author

jianchun commented Mar 4, 2016

Have you tried building in SD with these changes to see if char16 works?

I had trouble referencing wchar_t, but following worked so far (in Full):

#ifdef _WIN32
typedef WCHAR char16;
#define _u(s) L##s
#else
typedef char16_t char16;
#define _u(s) u##s
#endif

@tcare Anything in particular or steps you want me to try? (Send offline if not applicable here.)

@jianchun
Copy link
Author

jianchun commented Mar 5, 2016

See PR #460

@digitalinfinity
Copy link
Contributor

One other option which is worth considering is, we use char16_t everywhere since that's the standard type. And in the case of VS2013, we can typedef wchar_t to char16_t. What do others think? (We'll still need the _u macro).

@ianwjhalliday
Copy link
Collaborator

That's a good idea. I'm for trying that out.

@dilijev
Copy link
Contributor

dilijev commented Mar 6, 2016

That sounds like a good idea to me too. Then we would be one step closer to using all standard modern C++ types.

@obastemur
Copy link
Collaborator

+1 on char16_t _nix platforms. However isn't it better to use wchar_ on Windows ?

@jianchun
Copy link
Author

jianchun commented Mar 7, 2016

I'll try out char16_t. WCHAR is probably easier than wchar_t on VS2013.

@jianchun
Copy link
Author

jianchun commented Mar 7, 2016

I have tried char16_t.

On VS 2013 this works:

typedef WCHAR char16_t;
#define _u(s) L##s

On VS 2015, trying to use native char16_t runs into many issues as we suspected. Many APIs are based on WCHAR, LPCWSTR, LPWSTR, PWSTR, OLECHAR, LPCOLESTR. These types are based on native wchar_t and are not compatible with native char16_t. I can do following:

  • Stage 1: Behind the scene use WCHAR / L on VS 2015 for now. Because char16_t is a native type, I need to use #define to replace it.
#define char16_t WCHAR
#define _u(s) L##s
  • Stage 2: Switch to real native char16_t / u. Then we need type conversions from char16_t to WCHAR / wchar_t for all the APIs involved. (That will touch lots of code, but probably we need to do eventually. We can postpone that for some time.)
#define _u(s) u##s

On Unix we use PAL with which WCHAR / wchar_t are char16_t. So the related PAL implemented APIs accepts native char16_t / u and we don't have the issue on VS 2015.

@ianwjhalliday
Copy link
Collaborator

A little awkward. Stage 1 might be sufficient for a long time. Sounds reasonable to me.

@obastemur
Copy link
Collaborator

+1 on Stage1

How about defining wchar_t for VS2013 ?

#if defined(_MSC_VER) && _MSC_VER < 1900
#define WCHAR_IS_CHAR16  1 // in case we use char16_t explicitly we might need this
typedef wchar_t char16_t;
#endif

@jianchun
Copy link
Author

jianchun commented Mar 7, 2016

Thanks all for the discussion! I'll update the PR with char16_t / _u.

How about defining wchar_t for VS2013?

One issue with typedef to wchar_t on VS 2013 was we have internal build flavors that conflicts with it. wchar_t may or may not be a built in type on VS 2013 depending on a compiler switch. I found WCHAR always worked thus was using WCHAR. It is mostly equivalent to wchar_t on VS 2013.

@obastemur
Copy link
Collaborator

One issue with typedef to wchar_t on VS 2013 was we have internal build flavors that conflicts with it.

@jianchun that makes perfect sense. Thanks for details.

@jianchun
Copy link
Author

jianchun commented Mar 7, 2016

Just ran into a problem on VS 2015 with #define char16_t WCHAR and couldn't work around. We have some standard headers template-specialized on both wchar_t and char16_t. These are expected to be 2 different types.

Going back to char16...

@ianwjhalliday
Copy link
Collaborator

Bummer. Oh well, sounds good.

@dilijev
Copy link
Contributor

dilijev commented Mar 8, 2016

Just a note: according to the C++ standard, types ending in _t are reserved for future use by the standard even if they are not already defined. I don't believe there is any machinery in the compiler to override a builtin type so it would be ill advised to try. :P

@jianchun
Copy link
Author

jianchun commented Mar 8, 2016

Thanks all for the discussion and ideas. I have pushed PR #460 with char16/_u.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants