Skip to content

Conversation

@ringabout
Copy link
Member

No description provided.

@juancarlospaco
Copy link
Collaborator

Interesting. 👍

Araq pushed a commit that referenced this pull request Nov 5, 2024
ref #24225
related #24306

> Code in branches must not affect semantics of the code that follows
the
`when nimvm` statement. E.g. it must not define symbols that are used in
  the following code.

The test shouldn't have passed when
#24306
would be implemented somehow. Some third packages have already misused
`when nimvm` by defining symbols in the other branch of `when nimvm`.

e.g. in status-im/nim-unittest2#34

```nim
when nimvm:
  discard
else:
  let suiteName {.inject.} = nameParam

use(suiteName)
```
@ringabout ringabout changed the title test experimental:strictDefs enable experimental:strictDefs Nov 6, 2024
@ringabout ringabout marked this pull request as ready for review November 6, 2024 14:46
@Araq Araq merged commit 2df6331 into devel Nov 23, 2024
21 checks passed
@Araq Araq deleted the pr_strictdefs branch November 23, 2024 21:01
@github-actions
Copy link
Contributor

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

Hint: mm: orc; opt: speed; options: -d:release
177891 lines; 8.772s; 653.434MiB peakmem

narimiran pushed a commit that referenced this pull request Jan 14, 2025
ref #24225
related #24306

> Code in branches must not affect semantics of the code that follows
the
`when nimvm` statement. E.g. it must not define symbols that are used in
  the following code.

The test shouldn't have passed when
#24306
would be implemented somehow. Some third packages have already misused
`when nimvm` by defining symbols in the other branch of `when nimvm`.

e.g. in status-im/nim-unittest2#34

```nim
when nimvm:
  discard
else:
  let suiteName {.inject.} = nameParam

use(suiteName)
```

(cherry picked from commit c71de10)
Araq pushed a commit that referenced this pull request Jan 17, 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.

4 participants