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

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

Closed
quantimnot opened this issue Feb 27, 2023 · 1 comment · Fixed by #24135

Comments

@quantimnot
Copy link
Contributor

Description

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

Failing Examples

module.nim

discard """
matrix: "--backend:js doc"
"""
import std/jsffi

module.nims

# empty

Error

lib/system/jssys.nim(54, 1) Error: redefinition of 'getCurrentException'; previous declaration here: lib/system.nim(2430, 8)

module.nim

discard """
matrix: "doc"
"""
import std/jsffi

module.nims

--backend:js

Error

lib/system/jssys.nim(54, 1) Error: redefinition of 'getCurrentException'; previous declaration here: lib/system.nim(2430, 8)

Passing Examples

module.nim

discard """
matrix: "--backend:js doc"
"""
import std/jsffi

module.nims

Nim Version

1.6.10
```nim --version
Nim Compiler Version 1.6.10 [MacOSX: amd64]
Compiled at 2023-02-12
Copyright (c) 2006-2021 by Andreas Rumpf

active boot switches: -d:release
```
1.9.1-642136ec4f2cd93cdd753bf16fa4aec89b8fee82
```nim --version
Nim Compiler Version 1.9.1 [MacOSX: arm64]
Compiled at 2023-02-09
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 642136ec4f2cd93cdd753bf16fa4aec89b8fee82
active boot switches: -d:release -d:nimUseLinenoise
```

Current Output

No response

Expected Output

No response

Possible Solution

No response

Additional Information

No response

@metagn
Copy link
Collaborator

metagn commented Feb 27, 2023

Fix might be to change defined(js) to defined(js) and not defined(nimscript) here:

Nim/lib/system.nim

Lines 2219 to 2221 in 6fea221

when defined(js):
include "system/jssys"
include "system/reprjs"

Not defining js in nimscript is an option but I don't think there's any other way to tell if the current build is for javascript in a nimscript config file.

@github-actions github-actions bot added the stale Staled PR/issues; remove the label after fixing them label Jun 4, 2023
metagn added a commit to metagn/Nim that referenced this issue Sep 18, 2024
@metagn metagn removed the stale Staled PR/issues; remove the label after fixing them label Sep 18, 2024
@Araq Araq closed this as completed in 6cc50ec Sep 18, 2024
metagn added a commit to metagn/Nim that referenced this issue 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
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants