warn about destructors with `setLenUninit` in docs

[metagn]

The compiler normally prevents destructor calls when assigning to noinit locations (sink, variable init). But with setLenUninit the compiler can't track that the location is uninitialized, and so destructor calls are still generated. This can cause crashes since destructors may assume that the assigned location is well-formed. To deal with this, grow, add, setLen all use nodestroy to prevent any destructor calls from being generated. So the docs for setLenUninit now warn about this issue, and suggest using nodestroy.

[arnetheduck]

a compiler warning/error would be a lot more useful :/

[metagn]

I don't know if the compiler can easily track this, it's decided at runtime whether the memory is uninitialized or not. It's already unsafe to read from, it just might be hard to guess that it's unsafe to write to as well. The docs for nodestroy say it's temporary until a better solution, maybe whatever that is would make it more obvious that it needs to be used?

[arnetheduck]

I don't know if the compiler can easily track this

Well, the way I see it, newSeqUninit should only be exposed for what in C++ is called a trivial type - it's similar to supportsCopyMem but more nuanced - there are several such traits that would be useful in general, not just for newseq but for writing optimal serialization libraries and the like.

[Araq]

I'm tired of this uninit bullshit. Make the compiler use the DFA and stop exposing these nukes to Nim users.

[arnetheduck]

DFA

constraining uninit by traits is the way to only expose the non-nukes (and still allowing efficient serialization frameworks etc). dfa won't get you all the way regardless.

[Araq]

DFA is one thing and the other is better API design like newSeqWith(size, value) or maybe newSeqIt(4, it + 3) (which would produce @[3, 4, 5, 6]).

[arnetheduck]

newSeqWith(size, value)

what do you use to implement newSeqWith? or a deque? or any other custom collection? the traits are needed generally, not just for newSeqUninit because in order to implement serializers and collections, you need to cross the type/raw-byte boundary - the std lib does this using magics in a few places which constrains the design space of the language as a whole because libraries can't do it as efficiently, turning the type system into a bottleneck.

These facilities are not for "the general nim users" - they're primitives needed to build other safe abstractions.

[c-blake]

I think A) while there is a footgun docs should warn - so good PR & B) a proc newSeqUninit*[T: CopyMemable](...) (CopyMemable or similar name) to block the footgun at compile-time could be useful.

It hasn't yet been observed in this converation that the old newSeqUninitialized was overly constrained to [T: SomeNumber] (overly = blocking say a bool plain-old-data member types, etc.) which led me to add a crappy (but so constrained) newSeqNoInit in cligen/sysUt. So, I think [T: TheRightConstraint] may be a reasonable middle ground between "none of this BS at all" and "a totally unconstrained footgun".

Very parenthetically, it would be good for very "audit bait" things like this to be spelled one way, esp since the sub-substring "init" is also used all over the place, conventionally). I don't really care if it's {.noinit.} (back to 2011) or "uninit", but just one way, pretty please. (It's not Earth shattering or anything, but seems like needless, life-complicating variety.)

[github-actions[bot]]

This pull request is stale because it has been open for 1 year with no activity. Contribute more commits on the pull request and rebase it on the latest devel, or it will be closed in 30 days. Thank you for your contributions.

[arnetheduck]

still relevant - a doc warning is cute, but a compile-time warning for existing code + a trait is what's really needed here so we can implement Deque, Table and decode and so on safely and efficiently