Bug 162808

Summary:

JSStringRef should define JSChar without platform checks

Product:

WebKit

Reporter:

Nicolas Breidinger <Nicolas.Breidinger>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

annulen, commit-queue, keith_miller, mark.lam, msaboff, saam

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
JSStringRef change	none
updated patch	ggaren: review+
updated patch	none
updated patch	none

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	Details Formatted Diff Diff
updated patch (1.17 KB, patch) 2016-09-30 16:52 PDT, Nicolas Breidinger	ggaren: review+	Details Formatted Diff Diff
updated patch (1.24 KB, patch) 2016-09-30 19:50 PDT, Nicolas Breidinger	no flags	Details Formatted Diff Diff
updated patch (1.27 KB, patch) 2016-09-30 20:09 PDT, Nicolas Breidinger	no flags	Details Formatted Diff Diff
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.