1768634 - JSProtoKey macro won't compile on MSVC due to __VA_ARGS__ incompatibility

Status: UNCONFIRMED

(Core :: JavaScript Engine, defect, P3)

Description

[liamg_uw]

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

Steps to reproduce:

I am building SpiderMonkey for embedding in a C++ project. First I installed Mozilla Build and bootstrapped the entire mozilla-unified. Then I successfully ran js/src.configure.in, and then ran mozmake.exe and mozmake.exe install. Then I added the include folder to my project, linked mozjs-102a1.lib to my project, and copied mozjs-102a1.dll into my main project folder. Then I tried to compile my project in Visual Studio 2022.

Actual results:

The enum JSProtoKey in file jspubtd.h is created by a macro, but this macro won't compile. The error message says:

"syntax error: missing '}' before identifier 'JSProto_LIMIT'"

This doesn't provide very much information (that's the trouble with macros). I have written out the enum terms in long hand, and it compiles fine.

Expected results:

It should have compiled.

Comment 1

[Nicolas B. Pierron [:nbp — off until 29-09]]

My guess is that IF_RECORD_TUPLE macros are missing when ProtoKey.h is included. The macro is defined in TypeDecls.h.
Including TypeDecls.h before jspubtd.h should solve the issue.

Comment 2

[Tooru Fujisawa [:arai]]

Here's reduced testcase:

#define JS_FOR_EACH_PROTOTYPE(REAL) \
  REAL(AsyncIterator, AsyncIterator) \
  IF_RECORD_TUPLE(REAL(Tuple, Tuple))

#define IF_RECORD_TUPLE(x, ...) __VA_ARGS__

enum JSProtoKey {
#define PROTOKEY_AND_INITIALIZER(name, clasp) JSProto_##name,
  JS_FOR_EACH_PROTOTYPE(PROTOKEY_AND_INITIALIZER)
#undef PROTOKEY_AND_INITIALIZER
  JSProto_LIMIT
};

The preprocessed result differs between clang 14.0.3 and MSVC (tested on 2019).

clang:

enum JSProtoKey {

  JSProto_AsyncIterator,

  JSProto_LIMIT
};

MSVC:

enum JSProtoKey {

  JSProto_AsyncIterator

  JSProto_LIMIT
};

The comma expanded by previous macro REAL(AsyncIterator, AsyncIterator) is removed by __VA_ARGS__ in IF_RECORD_TUPLE(REAL(Tuple, Tuple)).

Comment 3

[Nicolas B. Pierron [:nbp — off until 29-09]]

Could this be handled as follow:

#define JS_FOR_EACH_PROTOTYPE(REAL) \
  REAL(AsyncIterator, AsyncIterator) \
  IF_RECORD_TUPLE(REAL, (Tuple, Tuple))

#define IF_RECORD_TUPLE(Macro, Args) Macro ## Args

enum JSProtoKey {
#define PROTOKEY_AND_INITIALIZER(name, clasp) JSProto_##name,
  JS_FOR_EACH_PROTOTYPE(PROTOKEY_AND_INITIALIZER)
#undef PROTOKEY_AND_INITIALIZER
  JSProto_LIMIT
};

When IF_RECORD_TUPLE is defined, it will concatenate the Macro name with the arguments which are expected.
See https://searchfox.org/mozilla-central/rev/da6a85e615827d353e5ca0e05770d8d346b761a9/js/src/jit/MacroAssembler.h#184-191 for a similar use case.

Comment 4

[Tooru Fujisawa [:arai]]

IF_RECORD_TUPLE is used in 2 ways, single argument for "if-then" and two arguments for "if-then-else",
that's why __VA_ARGS__ is there.
I guess, simple workaround is just to avoid using __VA_ARGS__, and split it into 2 macros,
or define dedicate macro for the prototype macro case.

Comment 5

[Tooru Fujisawa [:arai]]

This can be solved by dynamically generating the header in bug 1866644.