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

[request] setCap and getCap for strings and seqs #97

Open
emekoi opened this issue Aug 15, 2018 · 13 comments
Open

[request] setCap and getCap for strings and seqs #97

emekoi opened this issue Aug 15, 2018 · 13 comments

Comments

@emekoi
Copy link

emekoi commented Aug 15, 2018

would it be possible to add procedures for getting and setting the capacity of containers? you can already create strings and seq with a capacity, but there is currently no way of getting that capacity back.

@awr1
Copy link

awr1 commented Aug 15, 2018

Somewhat related: On the topic of newSeq/StringOfCap() I feel we should look towards deprecating these and just adding a capacity argument to newSeq/String() instead, and set it to a default value of -1 or something for auto-determination. IMO that would be cleaner and falls in line with the general desire to make system.nim smaller. Well, only sort of: I guess we would be adding two procs and (in the future) removing two procs...

@timotheecour
Copy link
Member

also, why not use the more natural way:

let cap = myseq.capacity
myseq.capacity = 10
myseq.capacity.inc

likewise with len (instead of setLen)

@Araq
Copy link
Member

Araq commented Aug 15, 2018

setLen is too expensive of a call for the x.len = y accessor syntax. I would also split that up into grow and shrink operations that better convey the meaning.

@emekoi
Copy link
Author

emekoi commented Aug 23, 2018

i was poking around nim's code and i stumbled upon core/seqs and in it grow and shrink. would these just need to be added to system.nim?

@Araq
Copy link
Member

Araq commented Aug 23, 2018

Yes I think so but that should probably an RFC of its own.

@emekoi
Copy link
Author

emekoi commented Aug 23, 2018

unrelated, but what is the purpose of the core modules?

@timotheecour
Copy link
Member

would these just need to be added to system.nim?

in the spirit only having strictly essential stuff in system.nim, why not another module? (eg sequtils or memory)

@narimiran narimiran transferred this issue from nim-lang/Nim Jan 13, 2019
@dom96
Copy link
Contributor

dom96 commented Mar 20, 2021

Just had a need for this, so +1 from me :)

A way to reverse engineer it is like so:

var x: seq[uint8]
for i in 0 ..< 3482:
  x.add(i.uint8)
echo(x.len)

type
  TGenericSeq = object
    len, reserved: int
echo(cast[ptr TGenericSeq](x).reserved)

@Yardanico
Copy link

Yardanico commented Mar 20, 2021

@dom96 this won't work for new runtime, just saying, a more generic code which works for both would be:

var x: seq[uint8]
for i in 0 ..< 3482:
  x.add(i.uint8)
echo(x.len)

when defined(nimV2):
  type
    SeqData = object
      cap: int
    SeqHeader = object
      len: int
      data: ptr SeqData
  proc getCap[T](x: seq[T]): int = 
    cast[SeqHeader](x).data[].cap

else:
  type
    SeqHeader = object
      len, cap: int
  
  proc getCap[T](x: seq[T]): int = 
    cast[ptr SeqHeader](x).cap

echo x.getCap()

@timotheecour
Copy link
Member

the question is where to add setCap and getCap.

setCap and getCap for strings and seqs could go under std/strbasics (ie, a low-level module so that it can be used in other modules without cyclic dependencies)

there is a lot of functionality shared between string and seq so it wouldn't make sense to duplicate API's in std/strbasics and a hypotehtical std/seqbasics, but I'm open for other suggestions.

@konsumlamm
Copy link

konsumlamm commented Mar 20, 2021

What's the problem with cap and cap=? Afaik, there is a cap field internally already, so it wouldn't be expensive.

@timotheecour
Copy link
Member

  • cap is indeed better than getCap
  • cap= is controversial, because it's not an O(1) call, for same reason we have len and setLen (but not len=); it's not a strict rule though because of cstring.len (not O(1))
  • note that there was some prior discussion on distinguishing API's for setLen when it results in a shrink vs expansion, but I don't remember the rationale
  • the question still remains of where to add cap and setCap, and i'd still suggest the slightly non-ideal std/strbasics unless someone has a better idea

@Araq
Copy link
Member

Araq commented Mar 23, 2021

but I don't remember the rationale

The rationale is that grow needs default(T) to exist and shrink doesn't.

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

No branches or pull requests

7 participants