Compilation in C++ fails when -Wpedantic and -Werror=unused-parameter flags are used

[slavslavov]

I am compiling the library with relaxed flags but I have stricter flags on my C++ code. And because my code includes quickjs.h I get the errors below

_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_NewBool(JSContext*, int)’:
_deps/quickjs-ng-src/quickjs.h:181:68: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                    ^
_deps/quickjs-ng-src/quickjs.h:483:12: note: in expansion of macro ‘JS_MKVAL’
  483 |     return JS_MKVAL(JS_TAG_BOOL, (val != 0));
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:181:75: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                           ^
_deps/quickjs-ng-src/quickjs.h:483:12: note: in expansion of macro ‘JS_MKVAL’
  483 |     return JS_MKVAL(JS_TAG_BOOL, (val != 0));
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:481:54: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  481 | static js_force_inline JSValue JS_NewBool(JSContext *ctx, JS_BOOL val)
      |                                           ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_NewInt32(JSContext*, int32_t)’:
_deps/quickjs-ng-src/quickjs.h:181:68: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                    ^
_deps/quickjs-ng-src/quickjs.h:488:12: note: in expansion of macro ‘JS_MKVAL’
  488 |     return JS_MKVAL(JS_TAG_INT, val);
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:181:75: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                           ^
_deps/quickjs-ng-src/quickjs.h:488:12: note: in expansion of macro ‘JS_MKVAL’
  488 |     return JS_MKVAL(JS_TAG_INT, val);
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:486:55: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  486 | static js_force_inline JSValue JS_NewInt32(JSContext *ctx, int32_t val)
      |                                            ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_NewFloat64(JSContext*, double)’:
_deps/quickjs-ng-src/quickjs.h:491:57: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  491 | static js_force_inline JSValue JS_NewFloat64(JSContext *ctx, double val)
      |                                              ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_NewCatchOffset(JSContext*, int32_t)’:
_deps/quickjs-ng-src/quickjs.h:181:68: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                    ^
_deps/quickjs-ng-src/quickjs.h:498:12: note: in expansion of macro ‘JS_MKVAL’
  498 |     return JS_MKVAL(JS_TAG_CATCH_OFFSET, val);
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:181:75: error: ISO C++ forbids compound-literals [-Wpedantic]
  181 | #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
      |                                                                           ^
_deps/quickjs-ng-src/quickjs.h:498:12: note: in expansion of macro ‘JS_MKVAL’
  498 |     return JS_MKVAL(JS_TAG_CATCH_OFFSET, val);
      |            ^~~~~~~~
_deps/quickjs-ng-src/quickjs.h:496:61: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  496 | static js_force_inline JSValue JS_NewCatchOffset(JSContext *ctx, int32_t val)
      |                                                  ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘int JS_IsBigInt(JSContext*, JSValue)’:
_deps/quickjs-ng-src/quickjs.h:533:46: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  533 | static inline JS_BOOL JS_IsBigInt(JSContext *ctx, JSValue v)
      |                                   ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_DupValue(JSContext*, JSValue)’:
_deps/quickjs-ng-src/quickjs.h:614:46: error: unused parameter ‘ctx’ [-Werror=unused-parameter]
  614 | static inline JSValue JS_DupValue(JSContext *ctx, JSValue v)
      |                                   ~~~~~~~~~~~^~~
_deps/quickjs-ng-src/quickjs.h: In function ‘JSValue JS_DupValueRT(JSRuntime*, JSValue)’:
_deps/quickjs-ng-src/quickjs.h:623:48: error: unused parameter ‘rt’ [-Werror=unused-parameter]
  623 | static inline JSValue JS_DupValueRT(JSRuntime *rt, JSValue v)

[saghul]

Patches are welcome for the first one but the code has a ton of unused parameters so it'll be tedious to get rid of all then I'm afraid!

[slavslavov]

This is the whole error log so there are only a few functions defined in the header file that cause this behaviour. The definitions can be either moved to the source file or the unused parameters to be casted to void - like (void)ctx;
The source file can be compiled with different flags, more relaxed, but the header is compiled with the flags I set to compile my source.

The bigger issue is with the macro - I am not an expert in that area and not sure what C++ will like.
If you suggest something for the macro, I can try it here in C++ and if it works I can push a PR (with casting the unused variables to void). One potential solution would be to conditionally define these macros - macro for C with the existing code and inline functions for C++

[slavslavov]

If I change the JS_MKVAL macro to the following:

#ifdef __cplusplus
    inline JSValue JS_MKVAL(int32_t tag, int32_t val) {
        JSValueUnion u;
        u.int32 = val;
        return JSValue{u, tag};
    }
#else
    #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
#endif

and cast the unused variables to void, the code now compiles.

But not sure do the rest of the macros like JS_MKPTR and JS_NAN need this too? Maybe they do not generate errors because they are currently not used in the code I am compiling.

[wuqi]

If I change the JS_MKVAL macro to the following:

#ifdef __cplusplus
    inline JSValue JS_MKVAL(int32_t tag, int32_t val) {
        JSValueUnion u;
        u.int32 = val;
        return JSValue{u, tag};
    }
#else
    #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
#endif

and cast the unused variables to void, the code now compiles.

But not sure do the rest of the macros like JS_MKPTR and JS_NAN need this too? Maybe they do not generate errors because they are currently not used in the code I am compiling.

#ifdef __cplusplus
#define JS_MKVAL(tag, val) JSValue{ JSValueUnion{ .int32 = val }, tag }
#define JS_MKPTR(tag, p) JSValue{ JSValueUnion{ .ptr = p }, tag }
#else
#define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
#define JS_MKPTR(tag, p) (JSValue){ (JSValueUnion){ .ptr = p }, tag }
#endif

Will these codes work?

[slavslavov]

@wuqi yes, this compiles and works but I think the issue is more complex. I tried to use one of the other macros to test it and I now get a different error message:

_deps/quickjs-ng-src/quickjs.h:173:43: error: use of old-style cast to ‘int32_t’ {aka ‘int’} [-Werror=old-style-cast]
  173 | #define JS_VALUE_GET_TAG(v) ((int32_t)(v).tag)
      |                                           ^~~

So I may need to handle this in a different way

[bnoordhuis]

Maybe a more pragmatic solution is to disable warnings inside the header file, e.g.

// at the top
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wunused-parameter"
// etc.
//...
// and at the bottom
#pragma GCC diagnostic pop

[bnoordhuis]

Once #615 lands (more accurately: if and when I figure out to invoke standalone cl.exe on the CI), I'll follow up with a pull request to make quickjs.h -Wunused-parameter clean.

[slavslavov]

Once #615 lands (more accurately: if and when I figure out to invoke standalone cl.exe on the CI), I'll follow up with a pull request to make quickjs.h -Wunused-parameter clean.

I am happy to test on Linux with my strict flags settings before you merge if you provide a commit hash.

[bnoordhuis]

The pull request is now open: #628

[bnoordhuis]

I just merged it. LMK if you still get build errors.

[slavslavov]

I tested the current master branch and my build with strict flags now passes. Thanks!