to panic or not to panic, that is the question

[gaissmai]

New() intercepts a panic in defer() of makeslice and at least for this reason New() is not inlineable and therefore the slice always escapes to the heap. Currently I work around this with From(buffer) but would it also be possible that a MustNew(int) is possibly inlineable if it just propagates the panic.

[lemire]

Sure, pull request invited.

[gaissmai]

maybe you will rethink if you really catch the panic in New() and change the length to 0 under the hood. Perhaps it's generally better to propagate the runtime panics unaltered. You can save some cpu cycles in the hot path and you panic anyway e.g. in extendSet and ShiftLeft .

[lemire]

@gaissmai Yes, although that would have to be a major release and we should carefully document that the constructor may panic. Pull request invited.

[gaissmai]

hmm, I'm not sure whether this requires a major release.

The public interface does not change, only the internal behavior:

• the possibility that New() is now panicking (nobody is happy with a suppressed panic)
• some other panic strings change from self crafted to runtime (the string is no public interface)

I would also leave out the test on panicIfNull, I would rather prefer the runtime panic. The bitset library is used for speed optimization and it is not a high level library, the users of this library prefer to see the runtime panic than a trapped and rewritten panic text.

request for comment

[bbrodriges]

the possibility that New() is now panicking (nobody is happy with a suppressed panic)

This is a big deal. For now there is no way function can panic from user perspective, but changing this behaviour without major version bump can lead to potential unexpected panics in runtime for users.

[gaissmai]

But there are other functions that trigger a panic for the same reason, i.e. if you have specified a tooBig start value for New(tooBig), then New does not panic (and unfortunately also returns no error) but possibly a Set(tooBig) later in the middle of the code.

[gaissmai]

and by the way, the current docstring for New() does not exclude any panic