mattreecebentley / plf_hive

plf::hive is a fork of plf::colony to match the current C++ standards proposal.
https://plflib.org/colony.htm
zlib License
71 stars 7 forks source link

Issues about comparison operators #3

Closed frederick-vs-ja closed 2 years ago

frederick-vs-ja commented 2 years ago

hive requires C++20, so all operator!=s can be removed.

hive::iterator etc. have operator<=>s returning int, which is not wrong, but makes std::compare_three_way()(i, j) ill-formed. It seems acceptable to have only one operator<=> instead of four "legacy" operator functions, like the following.

template <bool is_const_it>
inline std::strong_ordering operator <=> (const hive_iterator<is_const_it> &rh) const noexcept
{
    return element_pointer == rh.element_pointer ?
        std::strong_ordering::equal :
            group_pointer == rh.group_pointer ?
                std::to_address(element_pointer) <=> std::to_address(rh.element_pointer) :
                group_pointer->group_number <=> rh.group_pointer->group_number;
}

In P0447R19, hive's operator<=> is meaninglessly marked with constexpr. The constexpr should be removed.

mattreecebentley commented 2 years ago

hive requires C++20, so all operator!=s can be removed.

Do you mean for iterators as well? That would make a lot of perfectly reasonable code not work, if so. I have absolutely no intention of writing code which uses the spaceship operator within for loops which process iterators. Or (and forgive my ignorance here) does CPP20 automagickally create the other operators out of <=>? Even if so, a != operator using <=> as it's internal engine would be slower than a regularly-defined != operator, so there's no point removing it. != involves a single comparison, <=> involves multiple in this case.

hive::iterator etc. have operator<=>s returning int, which is not wrong, but makes std::compare_three_way()(i, j) ill-formed. It seems acceptable to have only one operator<=> instead of four "legacy" operator functions, like the following.

I admit that at this point, spaceship operator is beyond my comprehension, which also means that it is also beyond the comprehension of most users. The documentation devolves in to standardese very quickly.

template <bool is_const_it>
inline std::strong_ordering operator <=> (const hive_iterator<is_const_it> &rh) const noexcept
{
  return element_pointer == rh.element_pointer ?
      std::strong_ordering::equal :
          group_pointer == rh.group_pointer ?
              std::to_address(element_pointer) <=> std::to_address(rh.element_pointer) :
              group_pointer->group_number <=> rh.group_pointer->group_number;
}

Seems reasonable, will implement.

In P0447R19, hive's operator<=> is meaninglessly marked with constexpr. The constexpr should be removed.

Yes, that would appear to be a bug. Will fix in next version and upload before meeting.

frederick-vs-ja commented 2 years ago

Even if so, a != operator using <=> as it's internal engine would be slower than a regularly-defined != operator, so there's no point removing it. != involves a single comparison, <=> involves multiple in this case.

No. the != operator is only synthesized from operator== (as of C++20). So handwritten operator!= is almost no longer needed.

mattreecebentley commented 2 years ago

Ah, I see - thanks for explaining. Are >=, >, < and <= generally removed now and synthesized from <=>, too?

mattreecebentley commented 2 years ago

Nevermind, i see that they are - but unfortunately the synthesized codegen under gcc11 is slower and bulkier than the original specified code, so will retain the less-than/etc operators for the moment.