Open Bug 1768634 Opened 3 years ago Updated 10 months ago

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

Categories

(Core :: JavaScript Engine, defect, P3)

Product:
Core
Component:
JavaScript Engine
Version:
Firefox 102
Type:
defect

Tracking

()

Status:
UNCONFIRMED

People

(Reporter: liamg_uw, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

win-mozjs133-compile-errors.txt
100.89 KB, text/plain
Details

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.

Blocks: sm-meta
Severity: -- → S4
Component: General → JavaScript Engine
Priority: -- → P2
Priority: P2 → P3

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.

Flags: needinfo?(nicolas.b.pierron)
See Also: → 1769451

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)).

See Also: 1769451 →

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.

Flags: needinfo?(nicolas.b.pierron)

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.

Summary: JSProtoKey macro won't compile → JSProtoKey macro won't compile on MSVC due to __VA_ARGS__ incompatibility

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

See Also: → 1866644
Blocks: sm-build
No longer blocks: sm-meta

Hey,

this is still a problem!
I successful compiled SpiderMonkey as a shared lib on Windows and want to compile and link against it.

During compilation (with MSVC), I get the errors (in attachment win-mozjs133-compile-errors.txt).

The errors (some examples):

C:\tools\Windows-mozjs-133\include\fmt\base.h(444): error C2338: static_assert failed: 'Unicode support requires compiling with /utf-8'
...
C:\tools\Windows-mozjs-133\include\jspubtd.h(79): warning C4061: enumerator 'JSTYPE_LIMIT' in switch of enum 'JSType' is not explicitly handled by a case label
C:\tools\Windows-mozjs-133\include\jspubtd.h(37): note: see declaration of 'JSType'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2146: syntax error: missing '}' before identifier 'JSProto_SyntaxError'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2146: syntax error: missing ';' before identifier 'JSProto_Temporal'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2059: syntax error: ','
C:\tools\Windows-mozjs-133\include\jspubtd.h(89): error C2143: syntax error: missing ';' before '}'
C:\tools\Windows-mozjs-133\include\jspubtd.h(89): error C2059: syntax error: '}'
...
C:\tools\Windows-mozjs-133\include\js/Class.h(589): error C2065: 'JSProto_LIMIT': undeclared identifier
...
C:\tools\Windows-mozjs-133\include\js/Symbol.h(75): error C2146: syntax error: missing '}' before identifier 'Limit'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(76): error C2440: 'initializing': cannot convert from 'const uint32_t' to 'JS::SymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(76): note: Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or parenthesized function-style cast)
C:\tools\Windows-mozjs-133\include\js/Symbol.h(77): error C2440: 'initializing': cannot convert from 'unsigned int' to 'JS::SymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(77): note: Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or parenthesized function-style cast)
C:\tools\Windows-mozjs-133\include\js/Symbol.h(78): error C2440: 'initializing': cannot convert from 'unsigned int' to 'JS::SymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(78): note: Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or parenthesized function-style cast)
C:\tools\Windows-mozjs-133\include\js/Symbol.h(80): error C2440: 'initializing': cannot convert from 'unsigned int' to 'JS::SymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(80): note: Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or parenthesized function-style cast)
C:\tools\Windows-mozjs-133\include\js/Symbol.h(81): error C2143: syntax error: missing ';' before '}'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(84): error C2653: 'SymbolCode': is not a class or namespace name
C:\tools\Windows-mozjs-133\include\js/Symbol.h(84): error C2065: 'Limit': undeclared identifier
C:\tools\Windows-mozjs-133\include\js/Symbol.h(84): error C2737: 'WellKnownSymbolLimit': const object must be initialized
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C2146: syntax error: missing ';' before identifier 'GetSymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C2143: syntax error: missing ')' before '<'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C2143: syntax error: missing ';' before '<'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C2059: syntax error: '<'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(91): error C2059: syntax error: ')'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(99): error C2143: syntax error: missing ';' before '*'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(99): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\tools\Windows-mozjs-133\include\js/Symbol.h(100): error C2061: syntax error: identifier 'SymbolCode'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(99): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\tools\Windows-mozjs-133\include\js/Symbol.h(110): error C2059: syntax error: '}'
C:\tools\Windows-mozjs-133\include\js/Symbol.h(110): error C2143: syntax error: missing ';' before '}'
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(66): warning C4820: 'JSPropertySpec::ValueWrapper': '7' bytes padding added after data member 'JSPropertySpec::ValueWrapper::type'
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(195): warning C4820: 'JSPropertySpec': '6' bytes padding added after data member 'JSPropertySpec::kind_'
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(102): warning C4582: 'JSPropertySpec::Accessor::selfHosted': constructor is not implicitly called
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(101): warning C4582: 'JSPropertySpec::Accessor::native': constructor is not implicitly called
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(136): warning C4582: 'JSPropertySpec::AccessorsOrValue::value': constructor is not implicitly called
C:\tools\Windows-mozjs-133\include\js/PropertySpec.h(173): error C2039: 'PropertySpecNameIsSymbol': is not a member of 'JS'

The error exists in v128 and v133, when compiling with msvc.

Is there any solution or workaround?

As explained above, the issue comes from the __VA_ARGS__ handling in MSVC, and the workaround would be to rewrite the IF_RECORD_TUPLE macro not to use __VA_ARGS__.

Given we're not using MSVC in our automation, this has low priority and it's less likely this specific issue gets fixed soon.
In the long term, those headers will be replaced with dynamically-generated files, and that can solve it.

For short-term fix, if you're willing to submit a patch, I'm happy to review.

Alternative workaround would be to put a comma before JSProto_LIMIT, if you're going to use only MSVC (not clang or gcc).

(In reply to Tooru Fujisawa [:arai] from comment #8)

Alternative workaround would be to put a comma before JSProto_LIMIT, if you're going to use only MSVC (not clang or gcc).

You mean something like this (comma before JSProto_LIMIT):

/* Dense index into cached prototypes and class atoms for standard objects. */
enum JSProtoKey {
#define PROTOKEY_AND_INITIALIZER(name, clasp) JSProto_##name,
  JS_FOR_EACH_PROTOTYPE(PROTOKEY_AND_INITIALIZER)
#undef PROTOKEY_AND_INITIALIZER
      ,JSProto_LIMIT
};

That doesn't seem to help:

C:\tools\Windows-mozjs-133\include\jspubtd.h(79): warning C4061: enumerator 'JSTYPE_LIMIT' in switch of enum 'JSType' is not explicitly handled by a case label
C:\tools\Windows-mozjs-133\include\jspubtd.h(37): note: see declaration of 'JSType'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2146: syntax error: missing '}' before identifier 'JSProto_SyntaxError'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2146: syntax error: missing ';' before identifier 'JSProto_Temporal'
C:\tools\Windows-mozjs-133\include\jspubtd.h(86): error C2059: syntax error: ','
C:\tools\Windows-mozjs-133\include\jspubtd.h(89): error C2143: syntax error: missing ';' before '}'
C:\tools\Windows-mozjs-133\include\jspubtd.h(89): error C2059: syntax error: '}'

Maybe the situation has changed since this bug is filed?
Can you check the content of the preprocessed file? (I guess there's some equivalent of clang/gcc's -save-temps option)
The file will tell how the macro is expanded and what the actual syntax error is.

If I'm using the compiler switch correctly, I got a main.i file (shorten to JSProtoKey only):

enum JSProtoKey {

  JSProto_Null, JSProto_Object, JSProto_Function, JSProto_BoundFunction, JSProto_Array, JSProto_Boolean, JSProto_JSON, JSProto_Date, JSProto_Math, JSProto_Number, JSProto_String, JSProto_RegExp, JSProto_Error, JSProto_InternalError, JSProto_AggregateError, JSProto_EvalError, JSProto_RangeError, JSProto_ReferenceError   JSProto_SyntaxError, JSProto_TypeError, JSProto_URIError, JSProto_DebuggeeWouldRun, JSProto_CompileError, JSProto_LinkError, JSProto_RuntimeError, JSProto_ArrayBuffer, JSProto_Int8Array, JSProto_Uint8Array, JSProto_Int16Array, JSProto_Uint16Array, JSProto_Int32Array, JSProto_Uint32Array, JSProto_Float32Array, JSProto_Float64Array, JSProto_Uint8ClampedArray, JSProto_BigInt64Array, JSProto_BigUint64Array, JSProto_Float16Array, JSProto_BigInt, JSProto_Proxy, JSProto_WeakMap, JSProto_Map, JSProto_Set, JSProto_DataView, JSProto_Symbol, JSProto_ShadowRealm, JSProto_SharedArrayBuffer, JSProto_Intl, JSProto_Collator, JSProto_DateTimeFormat, JSProto_DisplayNames, JSProto_ListFormat, JSProto_Locale, JSProto_NumberFormat, JSProto_PluralRules, JSProto_RelativeTimeFormat, JSProto_Segmenter, JSProto_Reflect, JSProto_WeakSet, JSProto_TypedArray, JSProto_Atomics, JSProto_SavedFrame, JSProto_Promise, JSProto_AsyncFunction, JSProto_GeneratorFunction, JSProto_AsyncGeneratorFunction, JSProto_WebAssembly, JSProto_WasmModule, JSProto_WasmInstance, JSProto_WasmMemory, JSProto_WasmTable, JSProto_WasmGlobal, JSProto_WasmTag, JSProto_WasmFunction, JSProto_WasmSuspending, JSProto_WasmException, JSProto_FinalizationRegistry, JSProto_WeakRef, JSProto_Iterator, JSProto_AsyncIterator    JSProto_Temporal, JSProto_Duration, JSProto_Instant, JSProto_PlainDate, JSProto_PlainDateTime, JSProto_PlainMonthDay, JSProto_PlainYearMonth, JSProto_PlainTime, JSProto_TemporalNow, JSProto_ZonedDateTime   

      JSProto_LIMIT
};

So, IF_EXPLICIT_RESOURCE_MANAGEMENT macro is also hitting the same issue,
and the items after that lacks comma.

https://searchfox.org/mozilla-central/rev/df850fa290fe962c2c5ae8b63d0943ce768e3cc4/js/public/ProtoKey.h#88-91,153-158

REAL(ReferenceError, ERROR_CLASP(JSEXN_REFERENCEERR))                     \
IF_EXPLICIT_RESOURCE_MANAGEMENT(                                          \
    REAL(SuppressedError, ERROR_CLASP(JSEXN_SUPPRESSEDERR)))              \
REAL(SyntaxError, ERROR_CLASP(JSEXN_SYNTAXERR))                           \
...
REAL(AsyncIterator, OCLASP(AsyncIterator))                                \
IF_EXPLICIT_RESOURCE_MANAGEMENT(                                          \
    REAL(DisposableStack, OCLASP(DisposableStack)))                       \
IF_EXPLICIT_RESOURCE_MANAGEMENT(                                          \
    REAL(AsyncDisposableStack, OCLASP(AsyncDisposableStack)))             \
REAL_IF_TEMPORAL(Temporal, OCLASP(temporal::Temporal))                    \

So I suppose you need to address the macro definition for both IF_EXPLICIT_RESOURCE_MANAGEMENT and IF_RECORD_TUPLE, not to rely on __VA_ARGS__.

or, really-temporary workaround might be to remove those lines from ProtoKey.h file.

Oh, I think I found a solution for msvc (no change of the js source files)!

Adding /Zc:preprocessor /utf-8 as compiler flags seems to solve all problems (the /utf-8 flag is for another error))!

from cl.exe:

/Zc:arg1[,arg2] language conformance, where arguments can be:
... shorten ...
  preprocessor[-]       enable standard conforming preprocessor in C/C++
                        (on by default in C11 or later)

It seems, all is fine now. I will test a little bit more! Thanks for your help and quick response!

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

Attachment

General

Creator:
Created:
Updated:
Size: