search

flyx / NimYAML

YAML implementation for Nim
https://nimyaml.org
Other
191 stars 36 forks source link

replace shallowCopy for ARC/ORC #120

Closed ringabout closed 2 years ago

ringabout commented 2 years ago

shallowCopy has already been removed for ARC/ORC on the devel branch. The PR is my best effort to replace shallowCopy with similar functionality code.

Ref https://github.com/nim-lang/Nim/pull/19972

flyx commented 2 years ago

Thanks for this!

Is there some deadline when shallowCopy should be gone? Instead of having this quick fix, I'd like to do a more thorough refactoring of memory usage with sink/lent since they were not around when the code base was written. However if there are reasons for this to be merged quickly, I'll do it, since my available time is currently sparse.

ringabout commented 2 years ago

Thanks for your quick response!

Is there some deadline when shallowCopy should be gone?

Nim v2 won't support shallowCopy for ARC/ORC. Nim v2 probably comes the end of this year.

if there are reasons for this to be merged quickly,

My PR https://github.com/nim-lang/Nim/pull/19972 (defaults to ORC) tests whether important packages are compatible with ARC/ORC. I will disable tests for packages which don't support ARC/ORC or use refc to test it.

flyx commented 2 years ago

Okay, I'll merge this to be fit for Nim v2. Any improvement would be about small performance gain, which is low prio so I probably won't do it anytime soon.

Thanks again!