RESOLVED FIXED 162808
JSStringRef should define JSChar without platform checks 
https://bugs.webkit.org/show_bug.cgi?id=162808
Summary JSStringRef should define JSChar without platform checks
Nicolas Breidinger
Reported 2016-09-30 14:17:08 PDT
JSChar is typedef'd as either "unsigned short" or "wchar_t" based on a number of platform checks. This is problematic for non-standard ports. To solve this, a way to determine which typedef to use without platform checks is required.
Attachments
JSStringRef change (1.10 KB, patch)
2016-09-30 14:26 PDT, Nicolas Breidinger 		no flags
DetailsFormatted DiffDiff
updated patch (1.17 KB, patch)
2016-09-30 16:52 PDT, Nicolas Breidinger 		ggaren: review+
DetailsFormatted DiffDiff
updated patch (1.24 KB, patch)
2016-09-30 19:50 PDT, Nicolas Breidinger 		no flags
DetailsFormatted DiffDiff
updated patch (1.27 KB, patch)
2016-09-30 20:09 PDT, Nicolas Breidinger 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Nicolas Breidinger
Comment 1 2016-09-30 14:26:16 PDT
Created attachment 290384 [details] JSStringRef change
Geoffrey Garen
Comment 2 2016-09-30 15:57:17 PDT
Comment on attachment 290384 [details] JSStringRef change View in context: https://bugs.webkit.org/attachment.cgi?id=290384&action=review > Source/JavaScriptCore/API/JSStringRef.h:40 > +#if __WCHAR_MAX__ > 0x10000 Don't we need to check if __WCHAR_MAX__ is #defined?
Nicolas Breidinger
Comment 3 2016-09-30 16:39:16 PDT
(In reply to comment #2) > Don't we need to check if __WCHAR_MAX__ is #defined? Looking more closely at gcc and msvc, a better check would be as follows: > #if !defined(_NATIVE_WCHAR_T_DEFINED) && (!defined(__WCHAR_MAX__) || (__WCHAR_MAX__ > 0xffffU)) _NATIVE_WCHAR_T_DEFINED being msvc specific __WCHAR_MAX__ being gcc specific However, I'm not familiar with mac implementation, it seems according to apple's libc (https://opensource.apple.com/source/Libc/Libc-825.24/include/_types.h) it is not guaranteed to exist: > #ifdef __WCHAR_MAX__ > #define __DARWIN_WCHAR_MAX __WCHAR_MAX__ > #else /* ! __WCHAR_MAX__ */ > #define __DARWIN_WCHAR_MAX 0x7fffffff > #endif /* __WCHAR_MAX__ */
Nicolas Breidinger
Comment 4 2016-09-30 16:52:55 PDT
Created attachment 290405 [details] updated patch
Geoffrey Garen
Comment 5 2016-09-30 17:19:16 PDT
Comment on attachment 290405 [details] updated patch r=me
Nicolas Breidinger
Comment 6 2016-09-30 19:50:29 PDT
Created attachment 290419 [details] updated patch Covers a missing case for RVCT. Adds description for each set of compiler options.
Nicolas Breidinger
Comment 7 2016-09-30 20:09:51 PDT
Created attachment 290420 [details] updated patch
Mark Lam
Comment 8 2016-10-03 08:51:41 PDT
Comment on attachment 290420 [details] updated patch r=me
WebKit Commit Bot
Comment 9 2016-10-03 09:45:30 PDT
Comment on attachment 290420 [details] updated patch Clearing flags on attachment: 290420 Committed r206734: <http://trac.webkit.org/changeset/206734>
WebKit Commit Bot
Comment 10 2016-10-03 09:45:34 PDT
All reviewed patches have been landed. Closing bug.
Konstantin Tokarev
Comment 11 2016-10-03 09:46:58 PDT
ChangeLog entry does not match patch contents!
Mark Lam
Comment 12 2016-10-03 10:35:55 PDT
(In reply to comment #11) > ChangeLog entry does not match patch contents! How so?
Note You need to log in before you can comment on or make changes to this bug.