10

While perusing the Qt source code I came across this gem:

template <class T> inline T qgraphicsitem_cast(const QGraphicsItem *item)
{
    return int(static_cast<T>(0)->Type) == int(QGraphicsItem::Type)
        || (item && int(static_cast<T>(0)->Type) == item->type()) ? static_cast<T>(item) : 0;
}

Notice the static_cast<T>(0)->Type? I've been using C++ for many years but have never seen 0 being used in a static_cast before. What is this code doing and is it safe?

Background: If you derive from QGraphicsItem you are meant to declare an unique enum value called Type that and implement a virtual function called type that returns it, e.g.:

class Item : public QGraphicsItem
{
public:
  enum { Type = MAGIC_NUMBER };
  int type() const { return Type; }
  ...
};

You can then do this:

QGraphicsItem* item = new Item;
...
Item* derivedItem = qgraphicsitem_cast<Item*>(item);

This will probably help explain what that static_cast is trying to do.

Sisir
  • 4,584
  • 4
  • 26
  • 37
Rob
  • 76,700
  • 56
  • 158
  • 197

4 Answers4

8

This looks like a very dubious way to statically assert that the template parameter T has a Type member, and then verify its value is the expected magic number, like you state you are supposed to do.

Since Type is an enum value, the this pointer is not required to access it, so static_cast<Item>(0)->Type retrieves the value of Item::Type without actually using the value of the pointer. So this works, but is possibly undefined behavior (depending on your view of the standard, but IMO a bad idea anyway), because the code dereferences a NULL pointer with the pointer dereference operator (->). But I can't think why this is better over just Item::Type or the template T::Type - perhaps it's legacy code designed to work on old compilers with poor template support that couldn't work out what T::Type is supposed to mean.

Still, the end result is code such as qgraphicsitem_cast<bool>(ptr) will fail at compile time because bool has no Type member enum. This is more reliable and cheaper than runtime checks, even if the code looks like a hack.

AshleysBrain
  • 22,335
  • 15
  • 88
  • 124
  • 1
    Again, please evidence your assertion that it is "technically undefined behavior". See: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232 – p00ya Jun 08 '10 at 11:36
  • @p00ya: That is an _active_ defect. Its proposed resolution has not been agreed upon and has not been incorporated into the forthcoming C++0x final committee draft. Indirection through a null pointer (or dereferencing a null pointer--take your pick) results in undefined behavior. – James McNellis Jun 08 '10 at 15:11
  • 2
    Last I read there were some subtleties and debate over whether the pointer is dereferenced or not (eg. if `&*((int*)0)` is undefined due to the apparent dereference, even though it could be skipped - or calling a static member function through a null instance pointer, which is more like what's happening in the question). IMO the language lawyers can figure that out and us mortals can write code that doesn't look like it dereferences null pointers :) – AshleysBrain Jun 08 '10 at 15:39
  • 1
    The `&*((int*)0)` is valid in C99 (see my answer about that here: http://stackoverflow.com/questions/2896689/dereferencing-the-null-pointer/2896975#2896975). It would be valid in C++ were the resolution to the defect report linked by p00ya to be incorporated into the standard. I agree, though: if it looks wrong, try to find another way to do it :-P – James McNellis Jun 08 '10 at 15:45
5

It's a bit strange, yes, and is officially undefined behavior.

Maybe they could have written it as follows (note that T here is no more a pointer, whether it is a pointer in the original code):

template <class T> inline T * qgraphicsitem_cast(const QGraphicsItem *item)
{
    return int(T::Type) == int(QGraphicsItem::Type)
        || (item && int(T::Type) == item->type()) ? static_cast<T *>(item) : 0;
}

But they may have been bitten by constness, and forced to write 2 versions of the same function. Maybe a reason for the choice they made.

Didier Trosset
  • 36,376
  • 13
  • 83
  • 122
  • There are already two versions of this function (const and non-const). – Rob Jun 08 '10 at 08:42
  • Do you have any references for it being undefined behaviour? The static_cast in itself is fine (should be a null pointer or even call an (int) ctor). My interpretation of s5.2.5 in C++98 doesn't suggest to me that the member access is undefined either. – p00ya Jun 08 '10 at 08:53
  • 1
    @p00ya: the code is *dereferencing* the pointer. That *is* undefined behaviour. – Konrad Rudolph Jun 08 '10 at 08:56
  • 2
    @konrad: don't be so confident without reading the standard properly. For C, see http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232 – p00ya Jun 08 '10 at 10:49
  • @p00ya: Hm. That’s an amendment that’s not yet in action. Still, thanks for pointing this out. This would indeed make the action legal by §5.2.5/4. – Konrad Rudolph Jun 08 '10 at 11:41
  • @p00ya Wouldn't the cast to int and subsequent comparison cause it to be undefined behavior even under the new proposal? – Mark B Jun 08 '10 at 15:06
  • @p00ya: The proposal you linked to is 5 years old and has not yet been included in the C++0x draft as far as I can tell -- just saying. – sellibitze Jun 08 '10 at 15:14
  • @Mark B: No. The precise point of §5.2.5/4 (last bullet) along with the amendment is that `x->Foo` (`x` being a null pointer) resolves to `(*x).Foo` and that `*x` in itself is valid. Furthermore, `(*x).Foo` is also a valid rvalue if `Foo` is an enumerator (i.e. an `enum` constant). The implied rationale would be that we don’t actually have to access any object in order to read the value of `T::Foo`. – Konrad Rudolph Jun 08 '10 at 19:26
1

The current standard and the draft for the upcoming standard suggest that dereferencing a null pointer is undefined behaviour (see section 1.9). Since a->b is a shortcut for (*a).b the code looks like it tries to dereference a nullpointer. The interesting question here is: Does static_cast<T>(0)->Type actually constitute a null pointer dereference?

In case Type was a data member this would definitely be dereferencing a nullpointer and thus invoke undefined behaviour. But according to your code Type is just an enum value and static_cast<T>(0)-> is only used for scoping / name lookup. At best this code is questionable. I find it irritating that a "static type property" like a local enum value is accessed via the arrow operator. I probably would have solved it differently:

typedef typename remove_pointer<T>::type pointeeT;
return … pointeeT::Type … ;
sellibitze
  • 27,611
  • 3
  • 75
  • 95
1

This is a common trick to use protected(static) members from outside the (sub)class. A better way would have been to expose some method, but since it wasn't intended to be user class they let go the hard work?!!

Pkj
  • 11
  • 1