views:

89

answers:

5

I came across the following code recently:

class Foo
{
public:
    void bar();
    // .. other stuff
};

void Foo::bar()
{
    if(!this) {
        // .. do some stuff without accessing any data members
        return;
    }

    // .. do normal actions using data members
}

The code compiles because in C++ methods are just functions that are implicitly passed a pointer for 'this' and 'this' can be checked to be NULL just like any other pointer. Obviously this code is confusing and bad practice even though it doesn't crash; it would be pretty confusing to step through the code in the debugger, see a NULL pointer is about to have a method called on it and then not see the expected crash. My question is: does it violate the C++ standard to call SomeFooPtr->bar() where SomeFooPtr == NULL?

It occurs to me that it may not because the user defined operator-> returns a pointer, which means that even if that pointer is NULL it definitely hasn't been dereferenced (dereferencing a NULL pointer I'm sure is regarded by the standard as illegal or undefined). On the other hand the semantics of raw pointers don't necessarily have to match the semantics of user defined pointers -- perhaps operator-> on them is considered a dereference even though the compiler won't generate one.

A: 

It doesn't matter if it is legal, it is confusing to the reader. In the implementation where this code works, the vtable is being accessed by type, certainly not by object.

Moreover, I expect that this code was put in to cover for a constructor failure, which would mask a variety of problems elsewhere. Constructor failure should be handled correctly, not with a nasty kludge like the example.

msw
A: 

This is (all together now) undefined behavior. However, for many compilers it will work, with the additional constraint that the method must be non-virtual.

JSBangs
+1  A: 

It is UB. A good way to make it crash is to use it as a base class of a derived class that uses multiple inheritance. YMMV.

Hans Passant
+7  A: 

This will probably work on most systems, but it is Undefined Behaviour. Quoth the Standard:

5.2.5.3

If E1 has the type “pointer to class X,” then the expression E1->E2 is converted to the equivalent form (*(E1)).E2 [...]

And:

5.2.5.1

A postfix expression followed by a dot . or an arrow ->, optionally followed by the keyword template (14.8.1), and then followed by an id-expression, is a postfix expression. The postfix expression before the dot or arrow is evaluated;58) [...]

58) This evaluation happens even if the result is unnecessary to determine the value of the entire postfix expression, for example if the id-expression denotes a static member.

Evaluation of *x where x is a null pointer results in Undefined Behaviour, so yours is clearly a case of UB, before the function is even entered.

Thomas
Thanks, this is exactly what I was looking for.
Joseph Garvin
Sorry for the messy edits. First I overlooked something in the Standard, then I misread your question. It should be ok now.
Thomas
Could you give an example of an expression of the form `A.template B`? Where is this syntax relevant?
Philipp
@Philip: Say you are writing code inside a templated class that takes a T. Then within that class you make an object of some_other_class<T>. Then it turns out some_other_class<T> has a templated member function, foo. some_other_class<T> x; x.template foo();. Basically, when you feed a template parameter into another template, if the latter template itself has a templated member, you have to put the extra template keyword there when accessing it so as to not confuse the parser. At least I think. I do it intuitively now so I might not be describing it exactly right.
Joseph Garvin
@Philipp: What a coincidence, I only just learned about this syntax minutes ago! Here: http://stackoverflow.com/questions/3257890/how-to-fix-the-syntax-in-this-code-rife-with-templates
Thomas
@Joseph Garvin: That's a slightly confused explanation, particularly, in your case "some_other_class<T> x; x.template foo();", template keyword is not needed. Basically, you need the template keyword to distinguish between member variables and member templates of dependent types. Names in dependent types are interpreted as variables (or functions) by the parser so, if T is a template parameter (not actual type), eg. other_class<T>::foo<int>() would be interpreted as other_class<T>::foo is_less_than int... In this case, you'd use the template keyword to tell the compiler foo is a template.
jpalecek
+2  A: 

This test is broken even if dereferencing was not an UB. It breaks when this-adjustments for multiple inheritance come into the play:

#include <stdio.h>
class B
{
    int value;
    public:
    void foo()
    {
        if (!this)
            printf("this==0\n");
        else 
            printf("safe\n");
    }
};
class A { public: int anotherValue; };
class Z : public A,public B {};

int main()
{
    Z *z=0;
    z->foo();
}

prints "safe" here.

Luther Blissett