Project

General

Profile

Bug #1443

Serialization for std::unique_ptr Fails With Abstract Base Class

Added by Nils Deppe 9 months ago. Updated 19 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
02/22/2017
Due date:
% Done:

0%


Description

Attempting to serialize a std::unique_ptr fails when serializing an abstract base class with the error:

include/pup_stl.h:109:23: error: allocating an object of
      abstract class type 'EmbeddingMap<3, 3>'
        ptr.reset(new T);

This is because you are not able to call new on an abstract base class. One solution that we have used is:

template <typename T, typename std::enable_if<not std::is_base_of<
                          PUP::able, T>::value>::type* = nullptr>
inline void pup(PUP::er& p, std::unique_ptr<T>& t) {
  bool is_nullptr = nullptr == t;
  p | is_nullptr;
  if (not is_nullptr) {
    T* t1;
    if (p.isUnpacking()) {
      t1 = new T;
    } else {
      t1 = t.get();
    }
    p | *t1;
    if (p.isUnpacking()) {
      t.reset(t1);
    }
  }
}

template <typename T,
          typename std::enable_if<std::is_base_of<PUP::able, T>::value>::type* =
              nullptr>
inline void pup(PUP::er& p, std::unique_ptr<T>& t) {
  T* t1;
  if (p.isUnpacking()) {
    p | t1;
    t = std::unique_ptr<T>(t1);
  } else {
    t1 = t.get();
    p | t1;
  }
}

template <typename T>
inline void operator|(PUP::er& p, std::unique_ptr<T>& t) {
  pup(p, t);
}

I think the lower level issue might be that it only really makes sense to serialize an abstract base class if the receiving pointer is of the same concrete derived class.

History

#1 Updated by Eric Bohm 9 months ago

  • Assignee set to Eric Mikida

#2 Updated by Nils Deppe 7 months ago

This issue is present in the v6.8.0 beta. Could it be resolved before v6.8.0 final? I can make a pull request with the required changes if that'll help.

#3 Updated by Eric Mikida 7 months ago

If you post a patch we can definitely take a look and see if we can get it into 6.8.0

#4 Updated by Nils Deppe 7 months ago

I just realized this would require some C++11 things, std::enable_if and std::is_base_of. Rather than adding a temporary version of these I'd personally prefer to defer to 6.9.0, but if this is desired for 6.8.0 then there might be a workaround. Thoughts?

#5 Updated by Eric Mikida 7 months ago

  • Target version set to 6.9.0

I think deferring to 6.9 would be preferable.

#6 Updated by Nils Deppe 7 months ago

Alright, I'll try to get a patch in as quickly as possible for 6.9.0 then.

#7 Updated by Sam White 21 days ago

This needs to be revisited now that we're working on 6.9.0

#8 Updated by Nils Deppe 21 days ago

You should be able to just drop in our implementation into your code. We also have various other STL containers implemented here: https://github.com/sxs-collaboration/spectre/blob/develop/src/Parallel/PupStlCpp11.hpp

#9 Updated by Phil Miller 19 days ago

  • Description updated (diff)

(formatted code)

#10 Updated by Eric Mikida 19 days ago

Nils, I've pulled in the code you linked, as well as your definition of Requires, which is used heavily in your stl pup code. It seems to work without almost any modification, but I'm testing a few more things and should have a patch up for review tomorrow.

#11 Updated by Eric Mikida 19 days ago

  • Status changed from New to In Progress

#12 Updated by Nils Deppe 19 days ago

Hi Eric, that's great news! We use Requires instead of std::enable_if because upon failure it provides an "error message" in the form of the missing type alias. Feel free to use either Requires or switch to std::enable_if. I look forward to looking over the patch and seeing them land in Charm++!

Also available in: Atom PDF