Bug 179892

Summary:

Detect __declspec within JSBase.h

Product:

WebKit

Reporter:

Don Olmstead <don.olmstead>

Component:

JavaScriptCore

Assignee:

Don Olmstead <don.olmstead>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, darin, ews-watchlist, jfbastien, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	darin: review+
Patch	none

Don Olmstead

Reported 2017-11-20 11:29:09 PST

Clang has support for a Microsoft style __declspec attribute, https://clang.llvm.org/docs/LanguageExtensions.html#has-declspec-attribute, and this should be detected within JSBase.h.

Attachments
Patch (1.63 KB, patch) 2017-11-20 11:33 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (1.75 KB, patch) 2017-11-20 12:05 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (2.09 KB, patch) 2017-11-20 14:17 PST, Don Olmstead	darin: review+	Details Formatted Diff Diff
Patch (2.03 KB, patch) 2017-11-20 17:21 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Don Olmstead

Comment 1 2017-11-20 11:33:34 PST

Created attachment 327369 [details] Patch Uses clangs declspec detection. Also getting rid of WIN_CE define.

Don Olmstead

Comment 2 2017-11-20 12:05:24 PST

Created attachment 327370 [details] Patch

Don Olmstead

Comment 3 2017-11-20 14:17:02 PST

Created attachment 327375 [details] Patch Moving declaration to its own block. Adding RealView ARM Compiler support of declspec.

EWS Watchlist

Comment 4 2017-11-20 14:18:28 PST

Attachment 327375 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/JSBase.h:72: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/API/JSBase.h:72: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Darin Adler

Comment 5 2017-11-20 16:32:07 PST

Comment on attachment 327375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327375&action=review > Source/JavaScriptCore/API/JSBase.h:80 > +#ifndef __has_declspec_attribute > +#if defined(WIN32) || defined(_WIN32) || defined(__CC_ARM) || defined(__ARMCC__) > +#define __has_declspec_attribute(x) 1 > +#else > +#define __has_declspec_attribute(x) 0 > +#endif > +#endif Defining this macro isn’t great in a public header like JSBase.h, but I guess we can live with it. > Source/JavaScriptCore/API/JSBase.h:88 > +#elif (__has_declspec_attribute(dllimport) && __has_declspec_attribute(dllexport)) No need for these parentheses.

Don Olmstead

Comment 6 2017-11-20 17:21:35 PST

Created attachment 327380 [details] Patch Addressing feedback

EWS Watchlist

Comment 7 2017-11-20 17:23:08 PST

Attachment 327380 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/JSBase.h:72: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/API/JSBase.h:72: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Don Olmstead

Comment 8 2017-11-20 18:28:18 PST

Comment on attachment 327380 [details] Patch Apple Win failures are around FourCC which is unrelated

WebKit Commit Bot

Comment 9 2017-11-20 18:48:17 PST

Comment on attachment 327380 [details] Patch Clearing flags on attachment: 327380 Committed r225063: <https://trac.webkit.org/changeset/225063>

WebKit Commit Bot

Comment 10 2017-11-20 18:48:19 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 11 2017-11-20 18:50:26 PST

<rdar://problem/35651308>

Note You need to log in before you can comment on or make changes to this bug.