3

Xcode 6.3 (beta) gives me new warnings, any suggestions how to fix the problem? I don't think removing the check completely as void is a correct answer, since in some cases "camera" has been NULL and caused crashing later elsewhere.

Reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to false

Here's the code. How else could I verify camera exists?

Camera& camera = sceneEngine->camera();
// FIXME: this triggers an undefined-bool-conversion warning in Xcode 6.3 BETA
if (&camera == NULL) return;

And the related method from SceneEngine:

// HEADER
class SceneEngine
{
public:
    Camera& camera();
protected:
    Camera camera_;
}

// CPP
Camera& SceneEngine::camera() {
    return camera_;
}
JOM
  • 8,139
  • 6
  • 78
  • 111
  • 4
    If you have a `Camera&`, then the camera exists or you have undefined behaviour. No buts. – user253751 Feb 24 '15 at 06:22
  • So you suggest that to get rid of the warning I should just remove the check and handle crashing (when camera is NULL) in some other way? – JOM Feb 24 '15 at 06:26
  • 1
    References are never supposed to be `NULL`, they are meant to always point to an actual object, unless you are invoking undefined behaviour somewhere like forcing an object to exist at address 0 as demonstrated in R Sahu's answer. Either this assertion shouldn't be there or you should be using pointers instead. – Havenard Feb 24 '15 at 06:28
  • @JOM How could camera possibly be NULL? – user253751 Feb 24 '15 at 06:31
  • @immibis I was handed over a huge pile of code, still trying to figure it out :) This code was ok earlier, but LLVM with Xcode 6.3 is more strict. I like it, but this case is tricky. – JOM Feb 24 '15 at 06:38
  • 2
    @JOM you should probably find out where the camera might become NULL, and fix that. Alternatively, change the references to pointers throughout. – user253751 Feb 24 '15 at 06:42
  • In your case the compiler is simply telling you that the condition will be always be true like everybody said. However, IMO, there are other cases where this should be allowed (overaloaded `operator&`). `gcc` accepts this with no problem, and you can make `clang` accept it using the option `-Wno-undefined-bool-conversion`. – alfC May 31 '16 at 08:26

3 Answers3

8

Since the camera object is returned by reference, it cannot be null, since references always point to an actual existing object.

So if you take the address of a reference variable, it can't be null, since it's pointing to a valid object anyway.

Just use the return value without any null check.

PS. If the function returns a reference to a non-existing object (e.g. null), that's just undefined behavior.

Solution:

If camera() needs to be able to return null at some point, then change its return type to Camera* to be able to return a null pointer.

Else — camera() never returns null, i.e. it always returns a reference to a valid object —, your null check is not needed and that's what the error message is saying.

Emil Laine
  • 41,598
  • 9
  • 101
  • 157
  • You are right: either I have to change interface or make sure camera would always exist. I'll choose to change return type from Camera& to Camera* since the rest of code seems to expect that. Creating a missing camera would be a more risky task. Thanx for several solution ideas! – JOM Feb 24 '15 at 07:24
5

Since the language standard defines that you have undefined behavior in the moment that you have a NULL reference, the compiler is allowed to optimize away your condition entirely. Consequently, this code useless. The warning informs you about this.

To silence the warning, you need to write your code in a way that you ensure that you do not generate NULL references in the first place. I. e., before you write code like this:

void bar(Camera& cam);
void foo(Camera* cam) {
    bar(*cam);
}

you need to check the pointer before you convert it into a reference:

void bar(Camera& cam);
void foo(Camera* cam) {
    if(cam) bar(*cam);    //this works as desired
}
cmaster - reinstate monica
  • 38,891
  • 9
  • 62
  • 106
2

Looking at your line

if (&camera == NULL) return;

we can ask: When will that statement be true? The answer is only when camera is bound to a dereferenced NULL pointer.

If you had a clear case of bad code like:

Camera* cameraPtr = NULL;
Camera& camera = *cameraPtr;

the conditional of the above if statement will evaluate to true. If it is possible for camera to be bound to such a dereferenced pointer, the if statement has some value. Otherwise, it has no value and the compiler is warning you about that.

You asked:

How else could I verify camera exists?

When a function returns a reference, you have to assume that the function returns a valid Camera object and proceed to use it. Remove the if line.

R Sahu
  • 204,454
  • 14
  • 159
  • 270