CODEWITHSUNDEEP
c++c++11movestd-pairargument-unpacking
Display Name
Display Name

Reputation: 2403

std::pair assignment with downcast

For timeouts in event reactors and proactors I use a priority queue that also allows O(log(n)) random access removes of events (when the event is signalled/completes rather than a timeout occurring). I store std::pair<std::chrono::steady_clock::time_point, Timed *> where Timed is a class that adds has an index (pointing into the queue) to allow efficient removal when calling TimedQ::Remove(Timed *p). When I want to have an event type associated with a timeout, I derive from Timed. The queue's Top() and Pop() return a pair.

I used to have a a bunch of code using the queue such as

std::tie(timePt0, eventPtr0) = timeoutQ.Pop();
std::tie(timePt1, eventPtr1) = std::move(hold);

which worked fine before I started using a base class Timed * in the queue instead of specific event type (i.e. Timed was originally a templated type instead), as I eventually needed to support multiple different event types that can be associated with the timeouts. However, with eventPtr* being a derived type (that I can static_cast to from a Timed * returned by the queue), code like the above no longer works.

I'm wondering what's the best way to do this. Right now, it's ended up very verbose, and I'm concerned about efficiencies like temporaries being created as well:

auto v(timeoutQ.Pop());
timePt0 = v.first;
eventPtr0 = static_cast<TimedEvent *>(v.second);
std::tie(timePt1, eventPtr1) = std::move(std::make_pair(hold.first, static_cast<TimedEvent *>(hold.second)); // I didn't literally do it like this, but I'm just trying to illustrate my struggle

The only other idea I had was to template the functions that return a pair by the derived event class, but this seems poor from a code size perspective, as multiple instances of those functions will be created even though the machine code should be identical since in all cases it's a pointer that's stored.

Edit: I also tried using this, which compiles, but I'm not sure is correct or efficient:

template<class D>
std::pair<std::chrono::steady_clock::time_point, D *> &&Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed *> &&in)
{
    return std::make_pair(in.first, static_cast<D *>(in.second));
}

the initial example then would become

std::tie(timePt0, eventPtr0) = Cnvrt<std::remove_pointer<decltype(eventPtr0)>::type>(timeoutQ.Pop());
std::tie(timePt1, eventPtr1) = Cnvrt<std::remove_pointer<decltype(eventPtr1)>::type>(hold);

Upvotes: 1

Views: 332

Answers (1)

ildjarn
ildjarn

Reputation: 62985

The Cnvrt you've shown returns a dangling reference – classic UB.

Here's a corrected C++11-compliant version that also validates D at compile-time and removes the need for the manual std::remove_pointer<...>::type at the call site:

template<typename D>
constexpr
std::pair<std::chrono::steady_clock::time_point, D>
Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed*> const& in) noexcept
{
  static_assert(std::is_pointer<D>{}, "D is not a pointer type");

  using derived_type = typename std::remove_pointer<D>::type;
  static_assert(std::is_base_of<Timed, derived_type>{}, "D does not derive from Timed");

  using ptr_type = typename std::remove_cv<D>::type;
  return {in.first, static_cast<ptr_type>(in.second)};
}

// ...

std::tie(timePt0, eventPtr0) = Cnvrt<decltype(eventPtr0)>(timeoutQ.Pop());
std::tie(timePt1, eventPtr1) = Cnvrt<decltype(eventPtr1)>(hold);

Online Demo

Here is an implementation that should work on VC++ 2012:

template<typename D>
std::pair<std::chrono::steady_clock::time_point, D>
Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed*> const& in) throw()
{
  static_assert(std::is_pointer<D>::value, "D is not a pointer type");

  typedef typename std::remove_pointer<D>::type derived_type;
  static_assert(std::is_base_of<Timed, derived_type>::value, "D does not derive from Timed");

  typedef typename std::remove_cv<D>::type ptr_type;
  return std::make_pair(in.first, static_cast<ptr_type>(in.second));
}

Online Demo

There is no efficiency concern here whatsoever – even your worst-case scenario, if the compiler does no optimizations at all, is just a copy of one scalar and one pointer (VC++ 2012 may copy each twice, but again, only without optimizations enabled).

Upvotes: 1

Related Questions