Should I extend std::less for a comparison functor?

Question

I want to create a shared_ptr content-comparison functor to stand in for std::less<T> in associative containers and std algorithms. I've seen several examples of custom comparators that use the following (or similar) model:

template <typename T>
struct SharedPtrContentsLess {
  bool operator()(const boost::shared_ptr<T>& lhs, 
      const boost::shared_ptr<T> rhs) const {
    return std::less<T>(*lhs, *rhs);
    //or: return (*lhs) < (*rhs);
  }
  //defining these here instead of using std::binary_functor (C++11 deprecated)
  typedef boost::shared_ptr<T> first_argument_type;
  typedef boost::shared_ptr<T> second_argument_type;
  typedef bool return_type;
};

But why wouldn't I want to instead extend std::less? Like so:

template <typename T>
struct SharedPtrContentsLess : public std::less< boost:shared_ptr<T> > {
  bool operator()(const boost::shared_ptr<T>& lhs, 
      const boost::shared_ptr<T> rhs) const {
    return std::less<T>(*lhs, *rhs);
  }
};

Does this buy me anything at all?

I would think this gets me the typedefs for free, as though I was extending the deprecated std::binary_function. In C++03, I actually would be extending it through std::less. However, this would also be portable from C++03 to C++11/14 and even C++17 when std::binary_function will be removed, as it just follows the changes in std::less.

I've read a bunch of answers on StackOverflow regarding std::less use, custom comparison functors, and even some of the Standard specs and proposals. I see specializations of std::less and guidance not to extend STL containers, but I can't seem to find any examples of extending std::less or guidance against it. Am I missing an obvious reason not to do this?

EDIT: Removed C++11 tag, as it is causing confusion to the answerers. I am hoping to get forward-portability, but C++03 is required. If you provide a C++11-only answer for others to use (totally fine), please note that.

Is your code actually going to be compiled in a C++03 compiler? Is that worth any extra work? — Yakk - Adam Nevraumont, Aug 31 '15 at 20:22
@Yakk: Yes, currently, we're on GCC 4.4.7, which has very limited C++11 support. We'd like to update, but there are complicated reasons we haven't done so yet including security self-certification requirements and supporting programs that are stuck with the older compiler for now. What is the extra work here, though? The first version is what a C++11 version would look like, too, wouldn't it? — Bloodgain, Aug 31 '15 at 20:27
@Yakk: I think I understand what you mean now. I don't think the second version is more work at all. In fact, it seems like less work and the intent is more clear, which is the kind of thing we desire in code (i.e. elegance). It certainly seems like less code and more idiomatic C++ (in my opinion). If you think there's a reason the first version is preferable in either standard, please elaborate. That's what I'm asking, after all! — Bloodgain, Aug 31 '15 at 20:41
The less work is neither: don't do any typedefs at all, and do not inherit. Note that inheritance has a minor issues, in that slicing could occur. — Yakk - Adam Nevraumont, Aug 31 '15 at 20:43
Slicing between `SharedPtrContentsLess` and `std::less` or along the hierarchy of `T`? If it's the latter, I think I see the problem with both of these "generic" approaches. — Bloodgain, Aug 31 '15 at 20:52

score 1 · Answer 1 · answered Aug 31 '15 at 20:06

As you said in your question if you inherit from std::less the you would get the three typedefs that are in std::less. What I like most about inheriting from it is it describes your intent. When I see

struct some_non_specific_name : std::less<some_type>

I know right there that this is a functor that is going to behave as an < for some_type. I don't have to read the struct body to find out anything.

score 1 · Answer 2 · answered Aug 31 '15 at 20:12

As far as I can see, you are not missing any disadvantage. As you mentioned, you would automatically get the typedefs. The operator< has to be defined in both cases, and there is no difference in its implementation.

There is one thing that you might get that you might find neat, bad or just not applicable to your usecase (from here and here): there is a specialization of std::less for std::less<void> that has a template operator< deduces the return type of the operator< for the given arguments.

Unless you intend to use a SharedPtrContentsLess<void> (which probably doesn't make sense at all), both solutions would be equivalent.

Yakk - Adam Nevraumont · Answer 3 · 2015-08-31T20:59:05.563

I'd write a deref_less. First, my_less that smartly calls std::less:

struct my_less {
  template<class Lhs, class Rhs,
    class R = std::result_of_t< std::less<>( Lhs const&, Rhs const& ) >
    // class R = decltype( std::declval<Lhs const&>() < std::declval<Rhs const&>() )
  >
  R operator()(Lhs const&lhs, Rhs const&rhs)const{
    return std::less<>{}(lhs, rhs); // or lhs<rhs
  }
  // exact same type uses `std::less<T>`:
  template<class T,
    class R = std::result_of_t< std::less<>( T const&, T const& ) >
  >
  R operator()(T const& lhs, T const& rhs)const{
    return std::less<T>{}(lhs, rhs);
  }
  template<class Lhs, class Rhs,
    std::enable_if_t< std::is_base_of<Lhs, Rhs>{} && !std::is_same<Lhs, Rhs>{} >* = nullptr
  >
  bool operator()(Lhs const* lhs, Rhs const* rhs)const{
    return std::less<Lhs const*>{}(lhs, rhs);
  }
  template<class Lhs, class Rhs,
    std::enable_if_t< std::is_base_of<Rhs, Lhs>{} && !std::is_same<Lhs, Rhs>{} >* = nullptr
  >
  bool operator()(Lhs const* lhs, Rhs const* rhs)const{
    return std::less<Rhs const*>{}(lhs, rhs);
  }
  template<class Lhs, class Rhs,
    std::enable_if_t<
      !std::is_base_of<Rhs, Lhs>{}
      && !std::is_base_of<Lhs, Rhs>{}
      && !std::is_same<Lhs, Rhs>{}
    >* = nullptr
  >
  bool operator()(Lhs const* lhs, Rhs const* rhs)const = delete;
};

then, a deref_less that does a * then calls myless:

struct deref_less {
  template<class Lhs, class Rhs,
    class R = std::result_of_t< my_less( decltype(*std::declval<Lhs>()), decltype(*std::declval<Rhs>()) ) >
  >
  R operator()(Lhs const& lhs, Rhs const&rhs)const {
    return my_less{}( *lhs, *rhs );
  }
};

in C++14, but everything I used is easy to replace (std::less<> can be replaced with decltype and <s for example).

I don't fully understand this answer yet, but is all the stuff at the top (`my_less`) intended to solve the slicing problem in both of my versions? Is there a good way to solve this in C++03? — Bloodgain, Aug 31 '15 at 20:56
@BloodGain Not slicing, but rather that `<` on pointers isn't well behaved. On related pointers, I use `std::less` above to make it well behaved. When I posted this, your question was [tag:C++11]. Since then you have made it clear you lack a C++11 compiler, so this answer is relatively obsolete. — Yakk - Adam Nevraumont, Aug 31 '15 at 20:59
Yes, I added the C++11 tag because I was hoping this was forward-portable. I see now it introduced too much confusion, so I removed it. The question context asks about forward-portability, so that should be enough. Thanks! — Bloodgain, Aug 31 '15 at 21:09
@BloodGain If you need both C++11 and C++03, I'd include both tags. If you only need one, only include one. — Yakk - Adam Nevraumont, Aug 31 '15 at 21:16

Andrew · Accepted Answer · 2015-08-31T21:16:58.387

You can create a reusable template towards any dereferencable object (i.e. any (smart) pointer) by simply forwarding the call to std::less or any other comparable object.

// c++11
template<template<class> Op, class T> struct deref_mixin;

template<template<class> Op, class T>
struct deref_mixin {
   auto operator()(const T &l, const T &r) const
   -> decltype(std::declval<Op<T>>()(*l, *r)) {
      return Op<T>{}(*l, *r);
   }
};

template<template<class> Op>
struct deref_mixin<Op, void> {
   template<class T, class U>
   auto operator()(const T &l, const U &r) const
   -> decltype(std::declval<Op<T>>()(*l, *r)) {
      return Op<void>{}(*l, *r);
   }
};

template<class T> using less_deref = deref_mixin<std::less, T>;
template<class T> using greater_deref = deref_mixin<std::greater, T>;
template<class T> using my_comparator_deref = deref_mixin<my_comparator, T>;

// c++03
template<template<class> Op, class T>
struct deref_mixin {
   bool operator()(const T &l, const T &r) const {
      Op<T> op;
      return op(*l, *r);
   }
};
// Technically, the void template partial specialization isn't defined in c++03, but it should have been :)
template<template<class> Op>
struct deref_mixin<Op, void> {
   template<class T, class U>
   bool operator()(const T &l, const U &r) const {
      Op<void> op;
      return op(*l, *r);
   }
};

template<class T> struct less_deref : deref_mixin<std::less, T> {};

I just noticed in one of the comments that you have limited access to C++11 code. You can get rid of the return type determination and just use bool. That should satisfy any predicate use-case. — Andrew, Aug 31 '15 at 21:07
I decided to mark this answer as accepted, because it shows a better generic approach to what I want to do. I think the actual answer to the question is in one of Yakk's comments (danger of slicing). If someone decides to expand on that, I'll mark it as the most direct answer to the question, but I think this and Yakk's answer offer the best guidance. This one has a C++03 version, so it gets the nudge. — Bloodgain, Aug 31 '15 at 23:22

Bloodgain · Answer 5 · 2019-02-13T19:05:29.130

0

Because std::less lacks a virtual destructor (i.e. implicit destructor only), inheriting from it could technically lead to undefined behavior. Since neither type contains any data members, destruction should work no matter how the object is referenced, but the standard forbids polymorphic deletion through static destructors because it presents a strong possibility of problems (slicing, incomplete deletion) in most cases.

See this answer: Thou shalt not inherit from std::vector

edited Feb 13 '19 at 19:05

answered Feb 13 '19 at 18:34

Bloodgain

196
2
12

? Why should this (what?) be undefined behaviour? Could you elaborate, please? – Walter Feb 13 '19 at 18:43
Because inheriting publicly from a type without a virtual destructor allows for a situation in which delete states the behavior is not defined (per the referenced answer): From [expr.delete] in the C++0x FCD: In the first alternative (delete object), if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined. I'm certainly not claiming this is the *best* reason not to do it, but it seemed worth noting. – Bloodgain Feb 13 '19 at 18:52
I'm certainly happy to be corrected in my understanding, though. – Bloodgain Feb 13 '19 at 18:53
@Walter I have clarified my wording to try to be more precise. I'd still appreciate your feedback if you feel I'm in error here. – Bloodgain Feb 13 '19 at 19:06
Btw, this is not appropriate as an answer. It should have been a comment at most. – Walter Feb 13 '19 at 20:50
@Walter On that point we disagree, as I asked "Am I missing an obvious reason not to do this?" Going against the standard/introducing undefined behavior is a strong reason not to do something. I haven't changed my upvotes or selected answer, I simply added another reason not to do what I proposed. If you can offer criticism why my answer is inaccurate, though, I'd be happy to correct it or remove it. – Bloodgain Feb 13 '19 at 21:49

Should I extend std::less for a comparison functor?

5 Answers5