Re: [PATCH v1 1/4] lib: introduce BPS_TREE_EQUAL custom comparator

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Tue, Feb 05, 2019 at 02:58:36PM +0300, Kirill Shcherbatov wrote:
> Introduce a macro BPS_TREE_EQUAL for BPS TREE class. This makes
> possible to define custom comparators for stucture-based leafs.
> Previously, a C++ comparison operator "!=" override was used for
> this purpose. Due to the fact that we are not going to rework on
> C++ C-only components of Tarantool like memtx_tree, we needed a
> way to make complex structures comparisons using preprocessor.
> 
> Needed for #3961
> ---
>  src/lib/salad/bps_tree.h       | 74 +++++++++++++++++++++-------------
>  test/unit/bps_tree_iterator.cc | 15 ++++---
>  2 files changed, 56 insertions(+), 33 deletions(-)
> 
> diff --git a/src/lib/salad/bps_tree.h b/src/lib/salad/bps_tree.h
> index 8d1a62108..1419af3f7 100644
> --- a/src/lib/salad/bps_tree.h
> +++ b/src/lib/salad/bps_tree.h
> @@ -1035,6 +1035,10 @@ struct bps_leaf_path_elem {
>  	bps_tree_pos_t max_elem_pos;
>  };
>  
> +#ifndef BPS_TREE_EQUAL
> +#define BPS_TREE_EQUAL(a, b) (a == b)
> +#endif
> +

Funny thing, this is only needed for self checks and only used in unit
tests. The patch is unobtrusive so I guess it's OK.

Please move this macro to the place where other BPS_TREE macros are
defined and augment the definition with a comment.

Also, I'm not quite sure about the name, because if we have 'compare'
operation, why do we need to define 'equal' separately? What about
BPS_TREE_SELFSAeE or BPS_TREE_IDENTICAL?

Alternatively, we could define BPS_TREE_NO_DEBUG for each BPS tree
definition in the code. Then we wouldn't need this macro at all. I'm
inclined to think that would be the easiest way to deal with this
problem.