quickjs-ng / quickjs

QuickJS, the Next Generation: a mighty JavaScript engine
MIT License
691 stars 66 forks source link

Lots of warnings about precision conversion loss #369

Open saghul opened 2 months ago

saghul commented 2 months ago

Build output here since it's too big for a GH comment: https://gist.github.com/saghul/bc2aa4a4db058f4327ad1894bd360301

bnoordhuis commented 2 months ago

A number of them can be fixed like this:

diff --git a/quickjs.h b/quickjs.h
index 6c538a8..fc5ac81 100644
--- a/quickjs.h
+++ b/quickjs.h
@@ -35,7 +35,7 @@ extern "C" {
 #endif

 #if defined(__GNUC__) || defined(__clang__)
-#define js_unlikely(x)        __builtin_expect(!!(x), 0)
+#define js_unlikely(x)        ((int)__builtin_expect(!!(x), 0))
 #define js_force_inline       inline __attribute__((always_inline))
 #define __js_printf_like(f, a)   __attribute__((format(printf, f, a)))
 #define JS_EXTERN __attribute__((visibility("default")))
bnoordhuis commented 2 months ago

You know, having said that, maybe it's better to simply remove js_unlikely. It's hardly used and I'm somewhat skeptical it improves performance much, if at all.

neko-para commented 3 weeks ago

For reference, I've built quickjs by msvc, gcc and clang with some preset warning options. It generates a huge amount of warnings, and as my project enables Werror, I've added bunch of flags to disable them. Hope these flags could share some information.

if(MSVC)
    add_compile_options("/sdl-;/wd4146;/wd4820;/wd4100;/wd4267;/wd4018;/wd4242;/wd4244;/wd5250;/wd4295")
    add_compile_options("/wd4245;/wd5045;/wd5045;/wd4457;/wd4388;/wd4200;/wd4256;/wd4068;/wd4115;/wd4456")
    add_compile_options(
        "/wd4389;/wd4706;/wd4389;/wd4152;/wd4996;/wd4702;/wd4132;/wd4061;/wd4101;/wd4334;/wd4701;/wd4189;/wd4576;/wd4711;/wd4710;/wd4746"
    )
else()
    add_compile_options("-Wno-pedantic;-Wno-unused-parameter;-fPIC")
endif()