2

I'm storing into a container a list of pointers to an heterogeneous set of objects, using a string to identify them. Regarding the safety of this operation, I've already discussed here and I'm fine with that.

The problem is that when the class has multiple inheritance and I try to cast the same pointer to either one or the other super class, it seems that the second cast fails.

Here is an example:

#include <iostream>
#include <map>
using namespace std;

class A {
public:
  virtual void hello() = 0;
};

class B {
public:
  virtual void goodbye() = 0;
};

class C : public A,
          public B
{
public:
  void hello() override {
    std::cout << "C says: Hello World!" << std::endl;
  }
  void goodbye() override {
    std::cout << "C says: GoodBye World!" << std::endl;
  }
};

class D : public A {
public:
  void hello() override {
    std::cout << "D says: Hello World!" << std::endl;
  }
};

class E : public B {
public:
  void goodbye() override {
    std::cout << "E says: GoodBye World!" << std::endl;
  }
};

int main()
{
  std::map <std::string, void*> mymap;

  C c;
  D d;
  E e;

  mymap["C"] = (void*) &c;
  mymap["D"] = (void*) &d;
  mymap["E"] = (void*) &e;

  static_cast<A*>(mymap["D"])->hello();
  static_cast<B*>(mymap["E"])->goodbye();
  static_cast<A*>(mymap["C"])->hello();
  static_cast<B*>(mymap["C"])->goodbye();

  return 0;
}

The expected output would be:

D says: Hello World!
E says: GoodBye World!
C says: Hello World!
C says: GoodBye World!

but what I get instead is:

D says: Hello World!
E says: GoodBye World!
C says: Hello World!
C says: Hello World!

I don't even know how is that possible since I'm not even calling hello.

EDIT After understanding what has been discussed in the duplicate of and this page, I've ended up with this solution:

int main()
{
  std::map <std::string, void*> mymap;

  C c;
  D d;
  E e;

  mymap["C"] = static_cast<A*>(&c);
  mymap["D"] = static_cast<A*>(&d);
  mymap["E"] = static_cast<B*>(&e);

  static_cast<A*>(mymap["D"])->hello();
  static_cast<B*>(mymap["E"])->goodbye();
  static_cast<A*>(mymap["C"])->hello();
  dynamic_cast<B*>(static_cast<A*>(mymap["C"]))->goodbye();

  return 0;
}

An alternative solution that I've found is also making the second class inherit from the previous:

...
class B : public A{
public:
  virtual void goodbye() = 0;
};

class C : public B
{
public:
  void hello() override {
    std::cout << "C says: Hello World!" << std::endl;
  }
  void goodbye() override {
    std::cout << "C says: GoodBye World!" << std::endl;
  }
};
...
int main()
{
  std::map <std::string, void*> mymap;

  C c;
  D d;

  mymap["C"] = static_cast<A*>(&c);
  mymap["D"] = static_cast<A*>(&d);

  static_cast<A*>(mymap["D"])->hello();
  static_cast<A*>(mymap["C"])->hello();
  dynamic_cast<B*>(static_cast<A*>(mymap["C"]))->goodbye();

  return 0;
}
mcamurri
  • 153
  • 11
  • [Much smaller testcase with irrelevant things taken out](https://coliru.stacked-crooked.com/a/77272e768a094bad) - the lack of output is telling. There's UB here somewhere... – Lightness Races in Orbit Jul 17 '19 at 10:29
  • @LightnessRacesinOrbit, as discussed in the previous question, if the string is correct and I know exactly what I'm casting to, the operation is safe. This question is different since there are two superclasses now. I don't understand why if I cast the same pointer to either one or the other super class I don't get the correct function called. The code is of course much more complex than that and I didn't find a "well-defined approach" to the general problem I have, which is out of scope for this discussion anyways. – mcamurri Jul 17 '19 at 10:31
  • 1
    Perhaps this can help intuition: if you have `C c;` and you try to print the addresses of `(A*)&c` and `(B*)&c` you will get two different values, corresponding to the two subobjects. The compiler adjusted the pointer using the right offset since the compiler knows we are converting `C*` to `A*` and `B*`. If we move through `void *` we prevent the compiler to perform the adjustment -- hence the UB. If you cast `C*` to `void*`, you then need to cast it back to `C*`, after which you can cast again to `A*` and `B*`, if you want. – chi Jul 17 '19 at 12:40

1 Answers1

5

Your code has undefined behaviour.

Given a void* converted to B*, there is no way for the computer to know where (in the pointed-to thing that is actually a C) the B subobject is. You're just pretending that the whole thing (or, at least, a prefix of it) is a B, which would be true if it were a simple single inheritance… but yours isn't; the first base is an A, not a B.

Don't erase types with void* like this.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • I would add to this answer, that if you want to maintain the class inheritance structure, you might want to have a superclass that is the parents of all of them, so you could write `std::map mymap;`. Don't take this as a solution, it is an idea that might work. – VictorHMartin Jul 17 '19 at 11:44
  • Yes that is a good idea - add in some `dynamic_cast` for the win – Lightness Races in Orbit Jul 17 '19 at 11:58
  • 1
    @VictorHMartin I've already hoped for that solution with the Parent, but unfortunately the original classes are templated and there is no assumption that all the template parameter can have a common ancestor. I've just found [this question](https://stackoverflow.com/questions/24702235/c-stdmap-holding-any-type-of-value), it looks like what I'm trying to do is type erasure, so I would need to use `std::any`. Btw the original code is open source and is [here](https://github.com/ori-drs/pronto_core/blob/master/pronto_ros/include/pronto_ros/ros_frontend.hpp#L127). – mcamurri Jul 17 '19 at 12:56