3

In C++, assume following class hierarchy:

class BaseClass { };
class ChildClass : public BaseClass { };

Further assume factory classes for these two classes with a common, templated base class:

template<typename T>
class Factory {
public:
  virtual T* create() = 0;
};

class BaseClassFactory : public Factory<BaseClass> {
public:
  virtual BaseClass* create() {
    return new BaseClass(&m_field);
  }
private:
  SomeClass m_field;
};

class ChildClassFactory : public Factory<ChildClass> {
public:
  virtual ChildClass* create() {
    return new ChildClass(&m_field);
  }
private:
  SomeOtherClass m_field; // Different class than SomeClass
};

Note that the size/internal structure of ChildClassFactory and BaseClassFactory is different due to their different fields.

Now, if a have an instance of ChildClassFactory (or Factory<ChildClass>), can I safely cast it to Factory<BaseClass> (via reinterpret_cast)?

Factory<ChildClass>* childFactory = new ChildClassFactory();

// static_cast doesn't work - need to use reinterpret_cast
Factory<BaseClass>* baseFactory = reinterpret_cast<Factory<BaseClass>*>(childFactory);

// Does this work correctly? (i.e. is "cls" of type "ChildClass"?)
BaseClass* cls = baseFactory->create();

I know that you can't always cast templated classes this way, but in this special case a cast should be safe, shouldn't it?

I've tested it with Visual C++ 2010 and it does work. My question now is whether this is portable to other compilers?

Update: Since there has been some confusion let me clarify some more what's (supposed to be) important in my example:

  • ChildClass is a child class of BaseClass
  • A user of Factory<BaseClass> doesn't know what child class of BaseClass will be created. All he knows is that BaseClass is created.
  • Factory<T> has no fields of its own (other than the vtable).
  • Factory::create() is virtual
Sebastian Krysmanski
  • 8,114
  • 10
  • 49
  • 91
  • In this specific fragment of your code I see no errors. – Renan Greinert Jan 26 '12 at 13:33
  • Sorry, had to update it. – Sebastian Krysmanski Jan 26 '12 at 13:43
  • 5
    Note that your `BaseClassFactory` and your `ChildClassFactory` classes are not at all related in any way. Moreover, there's no point in having the `Factory` template, because each template instance is a distinct, unrelated type, so the virtual interface brings you no gain at all. – Kerrek SB Jan 26 '12 at 13:51
  • @Kerrek SB: Why doesn't it bring me anything at all? The code works as intended on Visual C++, so it can't be completely wrong. Also, since the call to `create()` is resolved at run time by using the vtable, it sure does bring me something. I can pass the factory instance with type `Factory` around and its user doesn't need to know which class ("other" than `BaseClass`) is actually created. – Sebastian Krysmanski Jan 26 '12 at 13:58
  • 1
    @SebastianKrysmanski C++ code happening to work doesn't mean it's not completely wrong. And saying it work *with Visual C++*, is even less meaningful. – R. Martinho Fernandes Jan 26 '12 at 14:00
  • 1
    @SebastianKrysmanski: When I do something like `T* t = new T; delete t; t->foo();` and it currently works on my current compiler, does that mean "it can't be completely wrong" here too? – PlasmaHH Jan 26 '12 at 14:07
  • @PlasmaHH: Bad example because it's clearly meant not to work. The question here is whether my code exploits undefined behavior or not. If it's UB then its not portable. But if it's defined behavior, I should be allowed to say "it working on my compiler". – Sebastian Krysmanski Jan 26 '12 at 14:20
  • @SebastianKrysmanski: NO, you are not getting what UB really means. When it is defined, it will work everywhere, not just in your compiler. When it is UB, it might or might not work anywhere. And clearly, your code invokes unspecified behaviour, as quoted in my answer below, which is here as worse as UB. You are trying to make "it works in my compiler" and argument for "it can't be UB", but you must then apply this everywhere, and can't just pick certain things out that you _want_ to work, and point to others and say "it clearly can't work because it is UB". – PlasmaHH Jan 26 '12 at 14:23
  • @PlasmaHH: Sure I know what UB means. At the I wrote the comment above I just wasn't yet convinced that I exploited UB. Oh, we have a different interpretation of what "clearly" means. – Sebastian Krysmanski Jan 26 '12 at 14:31
  • 1
    @SebastianKrysmanski: what I meant is that you can just remove the `Factory` template entirely, and you'll get the exact same behaviour. – Kerrek SB Jan 26 '12 at 15:31
  • @KerrekSB: But then I couldn't call `create()` without knowing the child class (`ChildClassFactory`, for example). But that's actually what I want. Maybe I shouldn't have used an example that is so similar to a well known design pattern. – Sebastian Krysmanski Jan 26 '12 at 16:08
  • @SebastianKrysmanski: Unless you actually have multiple, distinct factories for `ChildClass`, you have exactly one base class for every derived class, which is somewhat redundant. You might just specialize the factory class if that's all you need without going through the extra derived class. – Kerrek SB Jan 26 '12 at 16:47

3 Answers3

5

No, it is not. You may not use the result of a reinterpret_cast other than to cast stuff back, except for a few special cases:

ISO14882:2011(e) 5.2.10-7:

An object pointer can be explicitly converted to an object pointer of a different type.70 When a prvalue v of type “pointer to T1” is converted to the type “pointer to cv T2”, the result is static_cast(static_cast(v)) if both T1 and T2 are standard-layout types (3.9) and the alignment requirements of T2 are no stricter than those of T1, or if either type is void. Converting a prvalue of type “pointer to T1” to the type “pointer to T2” (where T1 and T2 are object types and where the alignment requirements of T2 are no stricter than those of T1) and back to its original type yields the original pointer value. The result of any other such pointer conversion is unspecified.

To make a possible failure scenario more clear, consider multiple inheritance, where using a static_cast or dynamic_cast would sometimes adjust the pointer value, but reinterpret_cast will not. Consider casting in this example from A* to B*:

struct A { int x; };
struct B { int y; };
struct C : A, B { };

To understand how your code fails in a different way too, consider how most compilers implement the virtual function call mechanism: With virtual pointers. Your instance of ChildClassFactory will have a virtual pointer, pointing to the virtual table of ChildClassFactory. Now when you reinterpret_cast this beast, it just happens to "work" incidentally, because the compiler expects some virtual pointer, pointing to a virtual table that will have the same/similar layout. But it will still contain the values pointing to the ChildCLassFactory virtual functions, thus these functions would be called. All of this is long after invoking undefined behaviour. It is as if you are jumping with a car into a large canyon and thinking "hey, everything is driving fine" just because you have not hit the ground yet.

PlasmaHH
  • 15,673
  • 5
  • 44
  • 57
  • @AndersK: I added some explanation about why his situation will fail too. – PlasmaHH Jan 26 '12 at 13:49
  • But `Factory` has no base class, so your example doesn't apply. – Sebastian Krysmanski Jan 26 '12 at 13:51
  • well I agree that @Sebastian Krysmanskis code does constitute undefined behavior, however I have yet to see a compiler where this code would fail to run. Not that I can claim to have tested all of them. So in my opinion the answer to the question "is it portable to other compilers?" is: probably, but it relies on behavior that is undefined in the standard. – PeterT Jan 26 '12 at 13:56
  • 1
    @PeterT: It is so heavily depending on things like virtual table layout, that you are on really extreme thin ice there. The function that is called by `baseFactory->create();` is `ChildClassFactory::create`. This call can quickly be destroyed by adding virtual functions, moving members, slightly changing some types. This is as portable as using inline assembler. – PlasmaHH Jan 26 '12 at 14:03
  • @PlasmaHH: I think we're getting somewhere here. So, the actual question is whether vtables of a template are always constructed exactly with exactly the same layout (e.g. the pointer to `create` is always the first one in the vtable) for all template instantiations. – Sebastian Krysmanski Jan 26 '12 at 14:14
  • @SebastianKrysmanski: You are already too far. vtables are an implementation detail, and a compiler may or may not use them; not to talk about how the compiler may or may not layout them. It could maybe even use a hashtable made out of the functions signature, who knows all the compiler internal details out there? It also depends on other things like that your compiler has the same bitpattern for the pointer of a derived and base class. That the compilers you used so far do it in such a way does not mean all do it, or that your compilers will continue to do it in the future. – PlasmaHH Jan 26 '12 at 14:28
  • @PlasmaHH: So, to sum things up: The information (vtable, ...) used to dispatch the virtual call `create()` may not be bit compatible for template instantiations created from the same template, right? – Sebastian Krysmanski Jan 26 '12 at 14:47
  • @SebastianKrysmanski: Yes, and it may not be bit compatible with any other type, as these are distinct types. – PlasmaHH Jan 26 '12 at 14:49
0

No, reinterpret_cast is only to be used for lowlevel code since it will not perfrom the correct address manipulation. Use static_cast or dynamic_cast instead,

Why do you want two factories this does not fit in the GoF factory pattern.

reinterpret_cast is not the way to do it since it is slow (runtime checks) and is not a nice OO design (you want to use the polymophisme build into the language).

Instead make constructors in the factory class that produces the types you are after and then have these call the constructor of the individual types.

The factory pattern allows you to be by ignorant of changes in your implementaion, which is a good thing since you minimize your dependencies, and allows for easier maintainance in the future of the code.

Damian
  • 4,395
  • 4
  • 39
  • 67
  • No, it is definitely not portable (Ok, maybe in his empty class example, but even then I don't think it's portable by standard, which is the only portability existing)! But neither is it slow, as `reinterpret_cast` does **not** do any runtime checks. In the processor it should be no-operation, the compiler just *reinterprets* the pointer. – Christian Rau Jan 26 '12 at 13:44
  • 1
    `reinterpret_cast` is not "slow (runtime checks)" as it is just a compile time hack telling the compiler to interpret a bitpattern differently. There is no runtime check involved. Also since `Factory` and `Factory` are two totally unrelated types, interpreting the bitpattern of one instance as another instance is not portable. It might work this time, just because they happen to be identical, but they are not. – PlasmaHH Jan 26 '12 at 13:45
  • @PlasmaHH: The aren't completely different. They use the same class, just different type arguments. The important thing here is that the `create()` method is virtual. Since `Factory` has no fields of its own (other than the vtable), there are no bit patterns to interpret. – Sebastian Krysmanski Jan 26 '12 at 13:49
  • 1
    @SebastianKrysmanski: You are having a misconception about templates and classes. A template is a template, not a class or a type. Only after instantiating it, it will become a type. But it is not in any way connected to types with other template parameters. `Foo` is a totally different type than `Foo`. This becomes clearer when you consider that it is possible to specialize on `Foo´ and make it _really_ something extremely different. – PlasmaHH Jan 26 '12 at 13:51
  • @PlasmaHH: That's why I wrote that I know that there are situations where you can't use reinterpret_cast. I just have `ChildClass` and `BaseClass` (and possibly other classes derived from `BaseClass`). – Sebastian Krysmanski Jan 26 '12 at 13:54
  • 1
    @SebastianKrysmanski: The inheritance relationships of template parameters is totally irrelevant here. Templates are just templates (in the standard english meaning of this term), as they provide a mechanism to create code. You can take your code and replace the derivation from `Factory` with a handwritten class `Factory_CC` (and the same for BC), both classes with the same body as the template. From the pov of C++, this won't change anything, but it might make it clearer that Factory_CC and Factory_BB are totally unrelated. – PlasmaHH Jan 26 '12 at 13:57
  • @PlasmaHH: Yes, but the point is that CC and BC have the same base class (or at least that's what my example should show). – Sebastian Krysmanski Jan 26 '12 at 14:00
  • 1
    @SebastianKrysmanski: So your point is that long after invoking UB you have the lucky case that you get a pointer that is incidentally having a usable bitpattern when interpreted as the wrong type in the current version of your compiler, with the current compile settings? Well, when this is your interpretation of `portable` so be it, but for most people in the world "relying on volatile behaviour of UB" != "portable" – PlasmaHH Jan 26 '12 at 14:06
  • @PlasmaHH: Not sure what you mean. What has any of this to do with the passing of time? – Sebastian Krysmanski Jan 26 '12 at 14:17
  • 1
    @SebastianKrysmanski The simple point is that in C++ a certain piece of code can work perfectly well on one compiler and crash (or even make the universe implode) on another compiler (or just the next version of your compiler). This is the case when your code invokes **undefined behaviour** and in this case your code **is wrong** or at least **not portable** (which is what you're looking for). – Christian Rau Jan 26 '12 at 14:24
  • @SebastianKrysmanski And once you realize that two template instantiations with different template parameters are completely unrelated types (no matter how the template parameters may be related), you will see that the `Factory` template is just useless, as there is no non-templated `Factory` base class, that would be a base class of both template instantiations, so virtual dispatching won't work the way you expect. – Christian Rau Jan 26 '12 at 14:28
  • @SebastianKrysmanski: It has to do with time that symptoms of invoking UB may or may not reveal themselve at the time UB is invoked; They may as well reveal themselve later, or not at all. – PlasmaHH Jan 26 '12 at 14:30
0

I've ticked the original answer above (to give him the credit), but I thought I'd sum up what I've learned here.

So, the basic problem is that it's not defined how dispatching virtual calls must be implemented.

This means that the data structure(s) that are internally used for virtual call dispatching (e.g. vtables) may or may not be bit compatible among template instantiations created from the same template.

Sebastian Krysmanski
  • 8,114
  • 10
  • 49
  • 91