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 system for nimscript config files on js backend #24135

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Sep 18, 2024

fixes #21441

When compiling for JS, nimscript config files have both defined(js) and defined(nimscript) be true at the same time. This is required so that the nimscript config file knows the current compilation is for the JS backend. However the system module doesn't account for this in some cases, defining JS-specific code or not defining nimscript-specific code when compiling such nimscript files. To fix this, have the nimscript define take priority over the js one.

@Araq Araq added the merge_when_passes_CI mergeable once green label Sep 18, 2024
@Araq Araq merged commit 6cc50ec into nim-lang:devel Sep 18, 2024
19 checks passed
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 6cc50ec

Hint: mm: orc; opt: speed; options: -d:release
174454 lines; 8.068s; 653.992MiB peakmem

metagn added a commit to metagn/Nim that referenced this pull request Sep 21, 2024
fixes nim-lang#21441

When compiling for JS, nimscript config files have both `defined(js)`
and `defined(nimscript)` be true at the same time. This is required so
that the nimscript config file knows the current compilation is for the
JS backend. However the system module doesn't account for this in some
cases, defining JS-specific code or not defining nimscript-specific code
when compiling such nimscript files. To fix this, have the `nimscript`
define take priority over the `js` one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge_when_passes_CI mergeable once green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc command on a module importing std/jsffi and using a module config file (<modulename>.nims) causes error
2 participants