fix #14655 setLen(seq) now zeros memory

[timotheecour]

• fix seq.setLen sometimes doesn't zero memory #14655
  1st commit is just a refactor
  2nd commit is the bugfix

after PR

implement proposal in #14655 (comment)

[timotheecour]

@Araq any word on proposal #14655 (comment) ?

[c-blake]

If you are going to zero by default (which is fine), you should either soon or someday provide a noInit=false that if flipped to true skips the zeroing. Then these setLens will work a lot like the regular variable decl with/without the noInit pragma..initialized by default with an optimization escape hatch. I realize the opposite isInit was proposed in that linked PR, but I think consistency with regular variables makes the most sense. There also seems value in having only one name (noInit) to remember. Just my two cents.

[timotheecour]

I'm fine with noInit, the only downside being that double negation is less readable than noInit. The important thing is we have that optimization escape hatch available.

[Araq]

@Araq any word on proposal #14655 (comment) ?

Somehow my reply got lost. Here is it again: I think an unsafe setLen is important but should be a different API.

[Araq]

Additional remark: A better design for setLen would be two separate procs; grow and shrink. Available with --gc:arc.

[timotheecour]

what would be benefits (besides the intent one where you call shrink if you expect it to shrink, but I haven't run into a case where this mattered)? honest question

[c-blake]

At the implementation level, less safe growNoInit (and maybe super unsafe shrinkNoDestruct) make sense. At the user-level, I think people mostly think about the length/legal addressable range. Nothing wrong with both grow/shrink and a wrapper setLen API.

While you might balk at shrinkNoDestruct, I have personally seen quite a few real life circumstances where it was taking an hour for a C++ program to quit() because giant arrays had to be swapped into RAM from spinning rust to run destructors. Swapping may/may not be preventable at an admin level, but any OS can always reclaim all (local) resources for a dying process in a far more efficient manner (where "far" could equate to "billions of times faster"). Something to consider.

I agree the double negative is slightly more cognitive load than ideal, but initialized is safer than uninitialized. So, we are stuck with var foo {.noInit.}. Cognitive load reduction from maximum Nim-global concept/naming consistency helps more than trying to "fix" it locally. Initted by default with an escape seems to be the concept/names we should stick with.

Similarly, whatever is done, it should be "similar", for newSeq[T](...), newSeqOfCap[T](...) which is why I thought "named parameter noInit=false" would be the simple way to go. setLenNoInit is not so bad and does force a clarity that the named parameter noInit=false would not (people could just pass "true" and then code readers might be WTF is that for? or even copy-paste errors). But consistency then means newSeqNoInit[T], newSeqOfCapNoInit[T] and all that. People could also just be "encouraged" to pass noInit=true not merely true.

All just food for thought. Not sure what the best answers are.

[Araq]

what would be benefits (besides the intent one where you call shrink if you expect it to shrink, but I haven't run into a case where this mattered)? honest question

You don't need default(T) to exist for shrinking but for growing.