Small shared two-way pointer

[danpoe]

This is a feature that is used in a future refactoring of sharing_mapt.

The class is similar to small_shared_ptrt and boost's intrusive_ptr. Like those, it stores the use count with the pointed-to object instead of in a separate control block. Additionally, it uses the MSB of the use count to indicate the type of the managed object (which is either of type U or V).

A possible use case is the implementation of data structures with sharing that consist of two different types of objects (such as a tree with internal nodes and leaf nodes). Storing the type with the use count avoids having to keep a separate type member or using typeid or dynamic_cast. Moreover, since the shared pointer is aware of the concrete type of the object being stored, it can delete it without requiring a virtual destructor or custom delete function (like std::shared_ptr).

[smowton]

This looks good, though I'm surprised we want something so specialised (use-count with the referee AND only two subtypes allowed). How much loss would it be to lift the latter restriction -- so you can have whatever subtypes you please, but require a vtable for destruction?

[danpoe]

I'd like to avoid having a vtable pointer in the managed object if not necessary as memory consumption is an issue in our work so we're trying to save wherever we can.

I think it would work though to support an arbitrary number of subtypes by using template parameter packs (kinda like std::tuple does). If people think this would be generally useful I can change the implementation to that.

[smowton]

How about the target (base) type has to (a) hold a refcount and (b) store some enum that provides enough information to fake a virtual delete operation, and it can do either as compactly as it likes? This pointer wouldn't then hard-code the implicit struct { unsigned refcount : 31; unsigned is_b_type : 1 }, but rather use nonvirtual ptr->nonvirtual_delete() (or some better name) for deletion and ptr->refcount, checking for overflow against the actual type of ptr->refcount, whatever the base type defines it to be?

Then if I'm incredibly tight for memory I can have { unsigned refcount : 7; unsigned is_b_type : 1 }, whereas if I have more possible base classes I can have { unsigned refcount : 24; unsigned char typeid; }.

I'm also free to be conventional or unconventional with regards to deletion:

Conventional style:

class my_normal_type {
  virtual ~my_normal_type();
  void nonvirtual_delete() {
    delete this;
  }
}

Avoiding a vptr but allowing a few subtypes:

class my_normal_type {
  void nonvirtual_delete() {
    switch(my_typeid) {
      case FOO:
        delete (foo*)this;
      case BAR:
        delete (bar*)this;
      case ...
    }
  }
}

[NlightNFotis]

Very nice. Just 2 minor comments, otherwise very solid. Thanks

[NlightNFotis]

Since we are building with C++11, nullptr is an instance of the type nullptr_t, which is implicitly cast to boolean. So I think just PRECONDITION(u) should suffice.

[danpoe]

I usually prefer to have the nullptr there as I find it slightly more readable. But I can change it if that's what's done in the rest of the codebase.

[NlightNFotis]

How does this work? Why if either of these is true, then are they the same? Isn't it the case that the object is in a compromised logical state if p == nullptr or that the function is semantically void if other.p == nullptr?

[tautschnig]

I would have expected: if(b1 || b2) return b1 == b2;

[danpoe]

The function checks whether both pointers are convertible to either U * or V *, so since nullptr can be converted to both we return true here. Maybe I should change the name of the function to is_same_type() or something like that. Any suggestions?

[tautschnig]

Yes, is_same_type sounds better to me.

[tautschnig]

Could this refer to U instead of using 1? Maybe just set_derived_u?

[danpoe]

Yep, that's probably more readable.

[tautschnig]

I would have expected p != nullptr && p->is_derived1()?

[danpoe]

This essentially checks whether the conversion static_cast<U *>(p) is valid so we also allow nullptr.

[tautschnig]

Ok, it would be good to add once sentence of documentation to make clear this is the expected behaviour. (I don't think I'd have inferred that from "is_derived" - it's more "can_convert".)

[tautschnig]

I would have expected: if(b1 || b2) return b1 == b2;

[tautschnig]

You need to make sure that 1 is of type Num as otherwise either the bit-shift may be undefined or you could get surprising results upon bitwise negation.

[tautschnig]

Shouldn't this go in the util/ subfolder?

[danpoe]

@smowton Yes, that's a good idea. I think we'd want to supply a variadic template that allows to plug in the possible subtypes, and then generates the deletion code (like in your switch statement above), accessor methods, and use count/typeid bitfield. That way the shared pointer logic would be encapsulated in this utility class. The accessors would then be similar to those of std::tuple, i.e. we'd have get_derived<0>(sp), get_derived<1>(sp), etc.

If possible, I'd like to leave this further generalization to a future PR and raise an issue for it now.