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

using int64 to pass cgo.Handle instead int #371

Closed
wants to merge 1 commit into from

Conversation

buke
Copy link
Owner

@buke buke commented Feb 8, 2025

@qiuchengxuan
Copy link

You can cast pointer to double using math.Float64frombits to avoid performance loss

@buke
Copy link
Owner Author

buke commented Feb 8, 2025

I don't think using math.Float64frombits is good idea, cause float64 struct like this:
63 62────────52 51─────────────────────────────────0
│ │ │ │
│ │ │←─────── Mantissa(52) ────────→│
│ │
│ │←── Exponent(11)

└── Sign(1)
only 52bits to store value.

and cgo.Handle is uint64(64-bit), and all bits are used to store unsigned integer value .

@qiuchengxuan
Copy link

qiuchengxuan commented Feb 8, 2025

It's not type cast, actually is reinterpret uint64 as float64, no actual bit changes

@buke
Copy link
Owner Author

buke commented Feb 8, 2025

val, err := ctx.Eval(`(proxy, fnHandler, ctx) => function() { return proxy.call(this, fnHandler, ctx, ...arguments); }`)

here using ctx.Eval to call js function , and cause I did not check quickjs c code ,I'm not sure whether quickjs does conversion

@qiuchengxuan
Copy link

Number in quickjs is either int32 or double, so your argument probably won't be converted, just need convert that double back to uintptr in goProxy

@buke
Copy link
Owner Author

buke commented Feb 8, 2025

yeah, and I found quicks support BigUint64 , https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.h#L549. I should using it instead string

@qiuchengxuan
Copy link

It's not easy to convert back

@buke buke force-pushed the feature/cgoHandleString branch from af651a1 to f35b42d Compare February 8, 2025 16:13
@buke buke changed the title using string to pass cgo.Handle instead int using int64 to pass cgo.Handle instead int Feb 8, 2025
@buke
Copy link
Owner Author

buke commented Feb 8, 2025

I think the original implementation using ctx.Int64 to pass cgo.NewHandle is correct and safe. QuickJS's JS_NewInt64 function ensures that the full 64-bit integer value is preserved without any loss of precision or conversion to floating-point. This approach maintains the integrity of the handle value throughout the JavaScript-Go interaction.

@buke buke closed this Feb 8, 2025
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.

2 participants