Bug 179892 – Detect __declspec within JSBase.h

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Don Olmstead]

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.

[Don Olmstead]

Created attachment 327369 [details]
Patch

Uses clangs declspec detection. Also getting rid of WIN_CE define.

[Don Olmstead]

Created attachment 327370 [details]
Patch

[Don Olmstead]

Created attachment 327375 [details]
Patch

Moving declaration to its own block. Adding RealView ARM Compiler support of declspec.

[EWS Watchlist]

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 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]

Created attachment 327380 [details]
Patch

Addressing feedback

[EWS Watchlist]

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 on attachment 327380 [details]
Patch

Apple Win failures are around FourCC which is unrelated

[WebKit Commit Bot]

Comment on attachment 327380 [details]
Patch

Clearing flags on attachment: 327380

Committed r225063: <https://trac.webkit.org/changeset/225063>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35651308>