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

stdlib: move varargsLen to macros.nim #307

Merged
merged 1 commit into from
May 13, 2022

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented May 12, 2022

It could only be implemented in system due to a misuse of a
compiler magic, making it dependent on a VM implementation detail.

Changes

  • varagsLen is now located in macros.nim and doesn't depend on a
    compiler magic anymore
  • the tvarargslen.nim test is moved to the macros category

The macro was originally added to system in order to make it available with no extra imports. There also was a discussion in the original PR about whether or not this macro should even be needed in the first place, but the PR was merged without the discussion concluding.

@saem
Copy link
Collaborator

saem commented May 12, 2022

Bors r+

bors bot added a commit that referenced this pull request May 12, 2022
307: stdlib: move `varargsLen` to `macros.nim` r=saem a=zerbina

It could only be implemented in `system` due to a misuse of a
compiler magic, making it dependent on a VM implementation detail.

### Changes
- `varagsLen` is now located in `macros.nim` and doesn't depend on a
 compiler  magic anymore
- the `tvarargslen.nim` test is moved to the `macros` category



Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
@zerbina zerbina force-pushed the varargslen-macros branch from f695df6 to b871fa5 Compare May 12, 2022 17:04
@bors
Copy link
Contributor

bors bot commented May 12, 2022

Canceled.

@zerbina
Copy link
Collaborator Author

zerbina commented May 12, 2022

I missed adding an import as well as a wrong white-space fix, sorry

@zerbina zerbina force-pushed the varargslen-macros branch from b871fa5 to 97efb88 Compare May 12, 2022 17:12
It could only be implemented in `system` due to a misuse of a
compiler magic, making it dependent on a VM implementation detail.

## Changes
- `varagsLen` is now located in `macros.nim` and doesn't depend on a
 compiler  magic anymore
- the `tvarargslen.nim` test is moved to the `macros` category
@saem
Copy link
Collaborator

saem commented May 13, 2022

Bors r+

@bors
Copy link
Contributor

bors bot commented May 13, 2022

Build succeeded:

@bors bors bot merged commit f845a5c into nim-works:devel May 13, 2022
@zerbina zerbina deleted the varargslen-macros branch May 13, 2022 14:58
@haxscramper haxscramper added the stdlib Standard library label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants