1907001 - Embedding with SpiderMonkey 128.x fails on Visual Studio due to __has_attribute

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[fanc999]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36

Steps to reproduce:

Using the latest Visual Studio 2019 (along with Rust, clang-cl for building SpiderMonkey):

• Build and install Spidermonkey 128.0 from Firefox-128.0/Firefox 128.0ESR, using mozilla-build.
• Install the Meson build system.
• Build and install GLib, gobject-introspection and its dependencies.
• Checkout the mozjs128 branch of https://gitlab.gnome.org/ptomato/gjs.git, which is the repository for GJS (GNOME JavaScript engine) that is now being ported for SpiderMonkey 128.x.
• Using Visual Studio 2019, run 'meson setup %HOMEPATH%\gjs --buildtype=debugoptimized --prefix=c:/gnome.build.unstable/vs14/x64 -Dpkgconfig.relocatable=true -Dprofiler=disabled -Dskip_dbus_tests=true'

Actual results:

The Meson configuration fails with the following error:

meson.build:291:4: ERROR: Problem encountered: A minimal SpiderMonkey program
could not be compiled or linked. Most likely you should build it with a

Checking into the logs, an error occurred when including mozilla/Attributes.h, where the error was: ...\mozjs-128\mozilla/Attributes.h(96): fatal error C1012: unmatched parenthesis: missing ')', and this points to using the defined(__clang__) && __has_attribute(...) construct

Expected results:

The configuration (and build) should have succeeded

Comment 1

[fanc999]

The workaround I had was to add the following before the first instance of the usage of defined(__clang__) && __has_attribute(...) construct:

#if defined (_MSC_VER) && !defined (__clang__)
#define __has_attribute(attr) 0
#endif

Comment 2

[Philip Chimento [:ptomato]]

To make diagnosis easier so you don't have to install and configure GJS, here's the "minimal program" that fails to compile with VS2019:

#include <js/Initialization.h>
int main(void) {
    if (!JS_Init()) return 1;
    JS_ShutDown();
    return 0;
}

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Officially we don't suppport building with MSVC since Firefox 63.

Unofficially, we may take a patch depending on how invasive it is.

Comment 4

[fanc999]

Attachment: Attributes.h: Define macro for checking CLang-specific attributes in a portable manner

Here's my proposed patch for mfbt/Attributes.h.

With blessings, thank you!

Comment 5

[fanc999]

(Please note that this is done with diff -u since I don't have the repo cloned as my connection here isn't that great, so I don't have a commit message there)

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Ok, I have create a patch and submitted it to try for testing; if that comes back clean then I will submit the patch on your behalf

https://treeherder.mozilla.org/jobs?repo=try&revision=5a557d878b677f75434b86e67d23bd96b54402dc

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1907001 - Add MOZ_HAS_CLANG_ATTRIBUTE r?glandium

Note this patch was provided by a contributor to fix MSVC builds of a SpiderMonkey embedding.

Comment 8

[fanc999]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #7)

Created attachment 9413320 [details]
Bug 1907001 - Add MOZ_HAS_CLANG_ATTRIBUTE r?#firefox-build-system-reviewers

Note this patch was provided by a contributor to fix MSVC builds of a SpiderMonkey embedding.

Thanks a bunch! Sorry, I was too used to the GNOME style (the space before the parens).

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit BugBot documentation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e688d6c5e17
Add MOZ_HAS_CLANG_ATTRIBUTE r=glandium

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4e688d6c5e17

Comment 12

[Philip Chimento [:ptomato]]

Comment on attachment 9413320 [details]
Bug 1907001 - Add MOZ_HAS_CLANG_ATTRIBUTE r?glandium

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes build failure on MSVC.
• User impact if declined: Embedders cannot build SpiderMonkey using MSVC.
• Fix Landed on Version: 130
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only touches preprocessor macros and only affects non-Clang builds. Any breakages caused by this patch would be expected to show up at compile time.

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9413320 [details]
Bug 1907001 - Add MOZ_HAS_CLANG_ATTRIBUTE r?glandium

Approved for 128.2esr

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/87055016f93b