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

Fix NAN definition in some environments #1840

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

92dev
Copy link

@92dev 92dev commented Jan 19, 2025

Usage of non-builtin NAN can trigger error:
"Initializer element is not a compile-time constant"

This happened to me with current code, compiled it on latest toolsets over windows 11
Basically the quickjs.c is using the NAN which is normally a function, as a compile-time constant

The bigger fix is to take the updated QuickJS engine from: https://github.com/quickjs-ng/quickjs

There is another option of using (double)0x7FF8000000000000 manually,
but I opted against anything that relies on magic-numbers that can be per-toolset

Usage of non-builtin NAN can trigger error:
"Initializer element is not a compile-time constant"
@92dev
Copy link
Author

92dev commented Jan 19, 2025

More context, here's the fix from the updated QuickJS repo
image
Vs. your current code:
image

you can see they opted for my less-preferred solution (using manual magic number)

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.

1 participant