quickjs icon indicating copy to clipboard operation
quickjs copied to clipboard

Fix JSValue casting

Open metarutaiga opened this issue 1 year ago • 4 comments

I found MSVC Compatibility https://github.com/bellard/quickjs/pull/16 But it modified too many codes

And I tried MSVC 2022 to compile it today. It just needs to be modified the JSValue casting because it's not standard C code.

If I want to build it with MSVC 2022, I need to add -std:c11 -experimental:c11atomics option and https://github.com/metarutaiga/quickjs/commit/10f9f7c31c1bb10d9b673d4d4f46e921a56e5853 only.

Yeah, I dont like MSVC too.

metarutaiga avatar May 23 '24 07:05 metarutaiga

It just needs to be modified the JSValue casting because it's not standard C code.

Can you clarify that is non-standard about it?

saghul avatar May 23 '24 07:05 saghul

It just needs to be modified the JSValue casting because it's not standard C code.

Can you clarify that is non-standard about it?

You can add -Wpedantic into Makefile.

 ifdef CONFIG_CLANG
   HOST_CC=clang
   CC=$(CROSS_PREFIX)clang
   CFLAGS+=-g -Wall -MMD -MF $(OBJDIR)/$(@F).d
+  CFLAGS += -Wpedantic
   CFLAGS += -Wextra
   CFLAGS += -Wno-sign-compare
   CFLAGS += -Wno-missing-field-initializers

Result:

./quickjs.h:675:12: warning: C99 forbids casting nonscalar type 'JSValue' (aka 'struct JSValue') to the same type [-Wpedantic]
    return (JSValue)v;
           ^        ~
./quickjs.h:684:12: warning: C99 forbids casting nonscalar type 'JSValue' (aka 'struct JSValue') to the same type [-Wpedantic]
    return (JSValue)v;
           ^        ~

metarutaiga avatar May 23 '24 07:05 metarutaiga

Ah, right. Does MSVC choke on it? On QuickJS-ng we somehow didn't run into this...

saghul avatar May 23 '24 08:05 saghul

Ah, right. Does MSVC choke on it? On QuickJS-ng we somehow didn't run into this...

Maybe MSVC is use C89 default.

metarutaiga avatar May 23 '24 08:05 metarutaiga