nil-safety in bart datastructure methods

[amalscale]

Hi, thank you for your work on this library. I noticed that bart datastructures such as Lite are a little inconsistent in how they treat nil receivers. It is safe to call some methods on nil receivers (e.g. MarshalJSON) but not others (e.g. Equal): https://go.dev/play/p/oldd3wkVGR6. This can cause some surprises when doing lazy initialization of a bart.Lite structure, for example.

My personal preference would be to make all methods nil-safe, but I'm open to learning more about why things are the way they are today. Thanks!

[gaissmai]

Lite, Bart, and Fast are designed to use the zero value. Additionally, achieving maximum speed for the basic operations Insert, Delete, and Lookup is the top priority. Having to test for nil every time in these operations costs time. You quickly run into issues when using a nil pointer, without the risk of getting incorrect results in that case. I'd say your example is artificial. But maybe I'm wrong.

[amalscale]

Additionally, achieving maximum speed for the basic operations Insert, Delete, and Lookup is the top priority. Having to test for nil every time in these operations costs time.

Fair enough, I can understand this decision for fastpath operations.

Is there a reason we shouldn't be nil-safe and support the commutative property for Equal (per example in OP)? There are valid reasons to avoid initialization (memory allocation) up-front in code that uses bart, and due to various go language decisions it is perfectly safe to call methods like Equal on a nil receiver. However, you can see the surprising outcome in the example where the order of operands affects the result.

[gaissmai]

I’ll take a closer look to see if I can improve it after all. Please remind me again in two weeks if you don’t see a new version on GitHub by then, since I’m currently in a busy phase working on the campus core migration and building a new data center in my paid job.

[gaissmai]

I’ve taken another closer look at the code and its dependencies, and I’ve now adjusted the code so that it’s coherent - meaning it triggers a panic whenever there’s a nil pointer, receiver, or argument, and there are no more hidden or undocumented exceptions.
Please take a look at the tip in the devel branch and let me know what you think.

[amalscale]

Thank you, I'll take a look sometime this week. I'm also happy to contribute PRs, but wanted to discuss things first.