Component::addChildComponent weird behavior

General JUCE discussion
1

It’s possible to call Component::addChildComponent without incident (at least on Visual Studio compiled code) on a null pointer, for example with :

// getParentComponent() returns nullptr
getParentComponent()->addChildComponent(mycompoment);

A code path is executed that doesn’t dereference the parent component’s members so the bug easily goes unnoticed. Maybe

jassert(this != nullptr); // better not add child components to a non-existing Component!

could be added to the addChildComponent implementation?

1 Like
2

Why wouldn’t you check if a function that returns a pointer is returning a valid pointer before using it?

if( auto* parent = getParentComponent() )
    parent->addChildComponent(...);
3

I had no reason to expect the pointer would be null in the case where I noticed this. (But it was, because of code that was wrong elsewhere.)

4

yeah, but it’s a fundamental rule to always assume a pointer is null by default. that’s why you check if its not before using it.

5

Isn’t that exactly why JUCE should be including those assertions in the library? :wink:

6

I would argue that this is such a basic C++ rule that no, this doesn’t require an assertion. Maybe a documentation update that says “this might return nullptr if it’s a TopLevelWindow” or something like that…

7

Maybe you misunderstood the problem. The problem isn’t about getParentComponent returning a null pointer and the documentation already mentions that can do it. The problem is that Component::addChildComponent happily “works” when used on an object instance dereferenced from a null pointer.

8

Oops, my mistake I read this backwards! Was thinking it was that the addChildComponent() method didn’t include nullptr assertions, which is the case:

I agree with you @matkatmusic that getParentComponent() shouldn’t be asserting if it’s going to return nullptr, it’s up to the caller to check the result before trying to use it.

How exactly is this happening? Is it not de-referencing anything due to the condition if (child.parentComponent != this) failing in the addChildComponent() method?

9

Right, it just returns from the method if that condition isn’t true.

10

I’m fairly certain in C++ that this can never be nullptr. Read another way, it’s not legal to call methods through a nullptr variable so the compiler is free to optimise anything that checks this != nullptr and assume it’s always true.

Basically, it’s best avoided and you have to do the checks yourself before making the addChildComponent call.

1 Like