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

Remove custom newSeqUninit if available in stdlib #592

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

Vindaar
Copy link
Collaborator

@Vindaar Vindaar commented Sep 28, 2023

Takes over from #591. We now use the stdlib newSeqUninit if available, i.e. after nim-lang/Nim#22739.

ringabout and others added 4 commits September 25, 2023 22:30
In these cases we are only dealing with mem copyable POD types, so
it's fine to use the stdlib version.
The proc is deprecated anyway and the minor performance penalty should
be irrelevant.
@Vindaar Vindaar merged commit bb6a90b into mratsim:master Sep 28, 2023
@ringabout
Copy link
Contributor

Hi, it doesn't seem to handle all the cases.

/home/runner/work/Nim/Nim/pkgstemp/arraymancer/src/arraymancer/autograd/autograd_common.nim(247, 19) template/generic instantiation of newSeqUninit from here
/home/runner/work/Nim/Nim/lib/system.nim(1659, 14) Error: The type T cannot contain managed memory or have destructors

ref nim-lang/Nim#22782

@Vindaar
Copy link
Collaborator Author

Vindaar commented Oct 2, 2023

Oh, I didn't realize these would be non mem copyable. I'll investigate and fix it!

edit: Ah, right, the seq here contains Tensor[<mem copyable>].

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.

2 participants