Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSVC compatibility #16

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

MSVC compatibility #16

wants to merge 53 commits into from

Conversation

c-smile
Copy link

@c-smile c-smile commented Oct 12, 2020

  1. General MSVC compatibility.
  2. CONFIG_VERSION (too common/generic) replaced by QUICKJS_VERSION due to clashes with other libraries that use CONFIG_VERSION too
  3. premake5 script - allows to generate makefile's, cmakes, VS project, XCode project, Code::Block, etc. Try premake5 gmake2 in sources directory.
    3.a premake5.lua script generates quickjs-version.h file from VERSION file.

1. MSVC compatibility
2. CONFIG_VERSION (too common/generic) replaced by QUICKJS_VERSION due to clashes with other libraries that use CONFIG_VERSION too
JS_STRICT_NAN_BOXING option allows compact packaging for 32 **and ** 64 bit versions.

JS_STRICT_NAN_BOXING is the only option that enables x64 builds using MSVC
@RGFTheCoder
Copy link

Send this to the mailing list. He doesn't accept pull requests

@c-smile
Copy link
Author

c-smile commented Oct 17, 2020

Umm... "this" here is what exactly? diff files ?

@andrewrch
Copy link

You can add .patch to a pull request URL to get a patch and then send that to the mailing list, e.g

https://github.com/bellard/quickjs/pull/16.patch

@past-due
Copy link

past-due commented Nov 1, 2020

@c-smile: 2ead907 seems to break QuickJS behavior - even in x86 / Win32 compiles, if JS_STRICT_NAN_BOXING is defined. (I'm trying to track down specifics now.) Any thoughts?

(I'm comparing a QuickJS x86 build with only 4298c1a applied, versus a QuickJS x86 build with 4298c1a and 2ead907 applied. The latter yields incorrect behavior for a complex series of scripts embedded in an open-source game.)

@c-smile
Copy link
Author

c-smile commented Nov 1, 2020

I do not see any problems yet with that variant of nan-boxing. I am using that variant in Sciter.JS. So far it runs Mithril and some other pretty big JS libraries as they are.

So I need more details.

@past-due
Copy link

past-due commented Nov 1, 2020

@c-smile: Thanks - was wondering if you'd come across any quirks. I will look into coming up with a minimal repro case when I have a chance.

In the interim, here is a potential alternative to your JS_STRICT_NAN_BOXING patch for MSVC 64-bit compatibility:

diff --git a/quickjs.c b/quickjs.c
--- a/3rdparty/quickjs/quickjs.c
+++ b/3rdparty/quickjs/quickjs.c
@@ -7212,7 +7212,7 @@ static int JS_DefinePrivateField(JSContext *ctx, JSValueConst obj,
         JS_ThrowTypeErrorNotASymbol(ctx);
         goto fail;
     }
-    prop = js_symbol_to_atom(ctx, (JSValue)name);
+    prop = js_symbol_to_atom(ctx, JS_VALUE_CONST_CAST(name));
     p = JS_VALUE_GET_OBJ(obj);
     prs = find_own_property(&pr, p, prop);
     if (prs) {
@@ -7243,7 +7243,7 @@ static JSValue JS_GetPrivateField(JSContext *ctx, JSValueConst obj,
     /* safety check */
     if (unlikely(JS_VALUE_GET_TAG(name) != JS_TAG_SYMBOL))
         return JS_ThrowTypeErrorNotASymbol(ctx);
-    prop = js_symbol_to_atom(ctx, (JSValue)name);
+    prop = js_symbol_to_atom(ctx, JS_VALUE_CONST_CAST(name));
     p = JS_VALUE_GET_OBJ(obj);
     prs = find_own_property(&pr, p, prop);
     if (!prs) {
@@ -7270,7 +7270,7 @@ static int JS_SetPrivateField(JSContext *ctx, JSValueConst obj,
         JS_ThrowTypeErrorNotASymbol(ctx);
         goto fail;
     }
-    prop = js_symbol_to_atom(ctx, (JSValue)name);
+    prop = js_symbol_to_atom(ctx, JS_VALUE_CONST_CAST(name));
     p = JS_VALUE_GET_OBJ(obj);
     prs = find_own_property(&pr, p, prop);
     if (!prs) {
@@ -7360,7 +7360,7 @@ static int JS_CheckBrand(JSContext *ctx, JSValueConst obj, JSValueConst func)
     if (unlikely(JS_VALUE_GET_TAG(obj) != JS_TAG_OBJECT))
         goto not_obj;
     p = JS_VALUE_GET_OBJ(obj);
-    prs = find_own_property(&pr, p, js_symbol_to_atom(ctx, (JSValue)brand));
+    prs = find_own_property(&pr, p, js_symbol_to_atom(ctx, JS_VALUE_CONST_CAST(brand)));
     if (!prs) {
         JS_ThrowTypeError(ctx, "invalid brand on object");
         return -1;
@@ -15954,7 +15954,7 @@ static JSValue js_call_c_function(JSContext *ctx, JSValueConst func_obj,
 #else
     sf->js_mode = 0;
 #endif
-    sf->cur_func = (JSValue)func_obj;
+    sf->cur_func = JS_VALUE_CONST_CAST(func_obj);
     sf->arg_count = argc;
     arg_buf = argv;
 
@@ -16198,7 +16198,7 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
     sf->js_mode = b->js_mode;
     arg_buf = argv;
     sf->arg_count = argc;
-    sf->cur_func = (JSValue)func_obj;
+    sf->cur_func = JS_VALUE_CONST_CAST(func_obj);
     init_list_head(&sf->var_ref_list);
     var_refs = p->u.func.var_refs;
 
@@ -38865,8 +38865,8 @@ static int64_t JS_FlattenIntoArray(JSContext *ctx, JSValueConst target,
         if (!JS_IsUndefined(mapperFunction)) {
             JSValueConst args[3] = { element, JS_NewInt64(ctx, sourceIndex), source };
             element = JS_Call(ctx, mapperFunction, thisArg, 3, args);
-            JS_FreeValue(ctx, (JSValue)args[0]);
-            JS_FreeValue(ctx, (JSValue)args[1]);
+            JS_FreeValue(ctx, JS_VALUE_CONST_CAST(args[0]));
+            JS_FreeValue(ctx, JS_VALUE_CONST_CAST(args[1]));
             if (JS_IsException(element))
                 return -1;
         }
@@ -40324,7 +40324,7 @@ static JSValue js_string_match(JSContext *ctx, JSValueConst this_val,
         str = JS_NewString(ctx, "g");
         if (JS_IsException(str))
             goto fail;
-        args[args_len++] = (JSValueConst)str;
+        args[args_len++] = JS_VALUE_MAKE_CONST(str);
     }
     rx = JS_CallConstructor(ctx, ctx->regexp_ctor, args_len, args);
     JS_FreeValue(ctx, str);
@@ -45354,7 +45354,7 @@ static JSMapRecord *map_add_record(JSContext *ctx, JSMapState *s,
     } else {
         JS_DupValue(ctx, key);
     }
-    mr->key = (JSValue)key;
+    mr->key = JS_VALUE_CONST_CAST(key);
     h = map_hash_key(ctx, key) & (s->hash_size - 1);
     list_add_tail(&mr->hash_link, &s->hash_table[h]);
     list_add_tail(&mr->link, &s->records);
@@ -45576,7 +45576,7 @@ static JSValue js_map_forEach(JSContext *ctx, JSValueConst this_val,
                 args[0] = args[1];
             else
                 args[0] = JS_DupValue(ctx, mr->value);
-            args[2] = (JSValue)this_val;
+            args[2] = JS_VALUE_CONST_CAST(this_val);
             ret = JS_Call(ctx, func, this_arg, 3, (JSValueConst *)args);
             JS_FreeValue(ctx, args[0]);
             if (!magic)
@@ -46554,7 +46554,7 @@ static JSValue js_promise_all(JSContext *ctx, JSValueConst this_val,
                 goto fail_reject;
             }
             resolve_element_data[0] = JS_NewBool(ctx, FALSE);
-            resolve_element_data[1] = (JSValueConst)JS_NewInt32(ctx, index);
+            resolve_element_data[1] = JS_VALUE_MAKE_CONST(JS_NewInt32(ctx, index));
             resolve_element_data[2] = values;
             resolve_element_data[3] = resolving_funcs[is_promise_any];
             resolve_element_data[4] = resolve_element_env;
@@ -46924,7 +46924,7 @@ static JSValue js_async_from_sync_iterator_unwrap_func_create(JSContext *ctx,
 {
     JSValueConst func_data[1];
 
-    func_data[0] = (JSValueConst)JS_NewBool(ctx, done);
+    func_data[0] = JS_VALUE_MAKE_CONST(JS_NewBool(ctx, done));
     return JS_NewCFunctionData(ctx, js_async_from_sync_iterator_unwrap,
                                1, 0, 1, func_data);
 }
@@ -52279,8 +52279,8 @@ static int js_TA_cmp_generic(const void *a, const void *b, void *opaque) {
             psc->exception = 1;
         }
     done:
-        JS_FreeValue(ctx, (JSValue)argv[0]);
-        JS_FreeValue(ctx, (JSValue)argv[1]);
+        JS_FreeValue(ctx, JS_VALUE_CONST_CAST(argv[0]));
+        JS_FreeValue(ctx, JS_VALUE_CONST_CAST(argv[1]));
     }
     return cmp;
 }
diff --git a/quickjs.h b/quickjs.h
--- a/3rdparty/quickjs/quickjs.h
+++ b/3rdparty/quickjs/quickjs.h
@@ -105,6 +105,8 @@ typedef struct JSRefCountHeader {
    for objective C) */
 typedef struct __JSValue *JSValue;
 typedef const struct __JSValue *JSValueConst;
+#define JS_VALUE_CONST_CAST(cVal) (JSValue)cVal
+#define JS_VALUE_MAKE_CONST(val) (JSValueConst)val
 
 #define JS_VALUE_GET_TAG(v) (int)((uintptr_t)(v) & 0xf)
 /* same as JS_VALUE_GET_TAG, but return JS_TAG_FLOAT64 with NaN boxing */
@@ -136,6 +138,8 @@ static inline JS_BOOL JS_VALUE_IS_NAN(JSValue v)
 typedef uint64_t JSValue;
 
 #define JSValueConst JSValue
+#define JS_VALUE_CONST_CAST(cVal) cVal
+#define JS_VALUE_MAKE_CONST(val) val
 
 #define JS_VALUE_GET_TAG(v) (int)((v) >> 32)
 #define JS_VALUE_GET_INT(v) (int)(v)
@@ -201,7 +205,7 @@ static inline JS_BOOL JS_VALUE_IS_NAN(JSValue v)
 typedef union JSValueUnion {
     int32_t int32;
     double float64;
-    void *ptr;
+    uintptr_t ptr;
 } JSValueUnion;
 
 typedef struct JSValue {
@@ -210,6 +214,8 @@ typedef struct JSValue {
 } JSValue;
 
 #define JSValueConst JSValue
+#define JS_VALUE_CONST_CAST(cVal) cVal
+#define JS_VALUE_MAKE_CONST(val) val
 
 #define JS_VALUE_GET_TAG(v) ((int32_t)(v).tag)
 /* same as JS_VALUE_GET_TAG, but return JS_TAG_FLOAT64 with NaN boxing */
@@ -217,10 +223,10 @@ typedef struct JSValue {
 #define JS_VALUE_GET_INT(v) ((v).u.int32)
 #define JS_VALUE_GET_BOOL(v) ((v).u.int32)
 #define JS_VALUE_GET_FLOAT64(v) ((v).u.float64)
-#define JS_VALUE_GET_PTR(v) ((v).u.ptr)
+#define JS_VALUE_GET_PTR(v) ((void *)((v).u.ptr))
 
 #define JS_MKVAL(tag, val) (JSValue){ (JSValueUnion){ .int32 = val }, tag }
-#define JS_MKPTR(tag, p) (JSValue){ (JSValueUnion){ .ptr = p }, tag }
+#define JS_MKPTR(tag, p) (JSValue){ (JSValueUnion){ .ptr = (uintptr_t)(void *)p }, tag }
 
 #define JS_TAG_IS_FLOAT64(tag) ((unsigned)(tag) == JS_TAG_FLOAT64)
 
@@ -675,7 +681,7 @@ static inline JSValue JS_DupValue(JSContext *ctx, JSValueConst v)
         JSRefCountHeader *p = (JSRefCountHeader *)JS_VALUE_GET_PTR(v);
         p->ref_count++;
     }
-    return (JSValue)v;
+    return JS_VALUE_CONST_CAST(v);
 }
 
 static inline JSValue JS_DupValueRT(JSRuntime *rt, JSValueConst v)
@@ -684,7 +690,7 @@ static inline JSValue JS_DupValueRT(JSRuntime *rt, JSValueConst v)
         JSRefCountHeader *p = (JSRefCountHeader *)JS_VALUE_GET_PTR(v);
         p->ref_count++;
     }
-    return (JSValue)v;
+    return JS_VALUE_CONST_CAST(v);
 }
 
 int JS_ToBool(JSContext *ctx, JSValueConst val); /* return -1 for JS_EXCEPTION */

@c-smile
Copy link
Author

c-smile commented Nov 1, 2020

Yes, that's what I started from. But realized that 128 bits for value is too much. Yet exactly half of them are used at any given time. Note that x64 uses 48 bits for address max.

Note that with JS_STRICT_NAN_BOXING JS_UNINITIALIZED value is exactly zero - uint64(0).
In original version uint64(0) is JSValue of type integer - int32(0).

Check if your code initializes JSValue storages properly.

@past-due
Copy link

past-due commented Nov 2, 2020

@c-smile: Please try the following Javascript with the current JS_STRICT_NAN_BOXING defined:

var valInfinity = Infinity;
var valLessThanInfinity = 0.5;
if (valLessThanInfinity < valInfinity) {
	console.log("Less than infinity");
} else {
	console.log("???"); // this is what I see output with JS_STRICT_NAN_BOXING defined
}

@c-smile
Copy link
Author

c-smile commented Nov 2, 2020

Oh, thanks a lot.

Fixed by c-smile@95e55eb

@past-due
Copy link

past-due commented Nov 2, 2020

👍

Yes, that's what I started from. But realized that 128 bits for value is too much. Yet exactly half of them are used at any given time. Note that x64 uses 48 bits for address max.

Also just a quick note, of which you may already be aware: Intel has announced PML5, yielding (potentially) 57-bit virtual addresses on x64 (if the OS chooses to support+enable it, on supported hardware).

@c-smile
Copy link
Author

c-smile commented Nov 2, 2020

Intel has announced PML5

On such systems, indeed, we would not worry about 128 bits for true | false and so it will be "plan B"

This change enables "named parameters call", consider this code
```
function foo(obj) {
  console.log(obj);
}

foo {bar:"bar"};
```

The above is equivalent of
```
foo ({bar:"bar"});
```
that is function call with single argument - object literal. Just tiread of writing those brackets....
<Foo /> - upper case tags are treated as functional or class components. So this
```
<Foo />
```
will lead to call :

JSX(Foo,{},[]);
JS_Eval2 is needed when JS is embedded into other files like HTML:

```
   <head>
       <script> // line 10 ( for example )
           // some script here
       </script>
```
so errors are reported properly.
@Alhadis
Copy link

Alhadis commented Mar 30, 2021

@c-smile The changes proposed here are huge and quite specific (JSX is not ECMAScript). IMHO, you're better off making a fork called quickjsx or something.

Moreover, @bellard has made it quite clear that patches are to be submitted to the mailing-list. As in, e-mailing this as a file attachment named msvc.patch or whatever. Though don't be surprised if 4.0 MBs worth of changes gets rejected…

@ekibun
Copy link

ekibun commented Apr 1, 2021

Hi, I've tried to add stack check for msvc as follow. It works fine in my case, but I'm not sure the replacement is correct.

static inline uintptr_t js_get_stack_pointer(void)
{
+ #ifdef _MSC_VER
+    return _AddressOfReturnAddress();
+ #else
    return (uintptr_t)__builtin_frame_address(0);
+ #endif
 }

see https://stackoverflow.com/a/22086803

cykoder pushed a commit to cykoder/quickjs that referenced this pull request Sep 1, 2023
feat: update test262.conf and addci yml
GerHobbelt pushed a commit to GerHobbelt/quickjs that referenced this pull request Oct 17, 2023
* Implements getQuickJSImmediate

* Update quickjs.ts

* Prettier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants