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

replace shallowCopy for ARC/ORC #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ringabout
Copy link

Hello, shallowCopy has been removed for ARC/ORC since it does a deep copy with ARC/ORC → nim-lang/Nim#20070

I don't know a good alternative for shallowCopy.

ref nim-lang/Nim#19972

@andreaferretti
Copy link
Owner

Hi @ringabout, thank you for the PR. But I must say I am confused - isn't there a mechanism to share storage with ORC? The whole point of the rotate function and RotatedString is to return something that behaves like a rotated string, but actually does not copy the whole memory.

@ringabout
Copy link
Author

ringabout commented Aug 31, 2022

isn't there a mechanism to share storage with ORC?

Imo, strings and seqs cannot be shared with ARC/ORC in a safe way. It can be either moved or copied. Sometimes it can be optimized using sink, which let the compiler choose whether to copy or move.

@ghost
Copy link

ghost commented Sep 6, 2022

I think the only workaround that can actually work in this case without changing semantics is creating some ref object type that stores the string internally (even though this will lead to double indirection which kinda sucks).

@planetis-m
Copy link

You could use a cow string implementation internally.

@andreaferretti
Copy link
Owner

I think I will go with double indirection as soon as I have time

@ringabout
Copy link
Author

I think the only workaround that can actually work in this case without changing semantics is creating some ref object type that stores the string internally (even though this will lead to double indirection which kinda sucks).

I'm not sure how to achieve that while keeping this test work

proc rotate*(s: string, i: int): RotatedString
test "underlying strings are shared":
  var
    x = "Hello, world"
    y = x.rotate(5)

  y[0] = 'f'
  check x[5] == 'f'

ringabout added a commit to nim-lang/cello that referenced this pull request Sep 21, 2022
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.

3 participants