Delete objects of incomplete type

Question

This one made me think:

class X;

void foo(X* p)
{
    delete p;
}

How can we possibly delete p if we do not even know whether X has visible destructor? g++ 4.5.1 gives three warnings:

warning: possible problem detected in invocation of delete operator:
warning: 'p' has incomplete type
warning: forward declaration of 'struct X'

And then it says:

note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.

Wow... are compilers required to diagnose this situation like g++ does? Or is it undefined behavior?

Orangeade

funny, came across this just yesterday! – Alert

Just for information: Visual C++ 9.0 shows such warning too... – Epicureanism

possible duplicate of Why, really, deleting an incomplete type is undefined behaviour? – Orangeade

Answer 1

From the standard [expr.delete]:

If the object being deleted has incomplete class type at the point of deletion and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined.

So, it's UB if there's nontrivial stuff to do, and it's ok if there isn't. Warnings aren't neccessary for UB.

Woodwaxen

No, it's not "UB if", it's unconditional UB. For example that class could have operator new overloaded on a separate heap and delete statement will now call wrong operator delete. – Widgeon

I stroked out the ambiguous part. As far as I can see, the standard doesn't say that delete-ing objects of incomplete type is UB in every case, just as mentioned in the section that I quoted. Why do you think that it is UB unconditionally? (Where does the standard say this?) – Woodwaxen

@etarion: The standard says that behavior will depend on how that class is declared which means that you can start with a class satisfying those requirements and then change it to not satisfy those requirements and now you face UB (which can be "it works" by the way). So although formally you're clean you've planted a fatal error into your code. The warning in question should be addressed - deleteing incomplete classes is a very bad idea. – Widgeon

So it IS actually conditional UB, and not unconditional as you claimed. (I'm not arguing if it's good practice or not. The question in the OP was if a diagnosis is required - no - and if it's UB - that depends.) – Woodwaxen

I will +1 this answer if you un-cross-out the "nontrivial" bit. It's exactly right - you can delete "something" of incomplete type provided that whoever eventually completes that type ensures it has trivial destruction and deallocation. Hopefully this will be documented somewhere, or perhaps that "whoever" is you. It's no more "unconditional UB" that it is "unconditional UB" to pass a null pointer into a forward-declared function. After all, the function could dereference it. But if the docs say it doesn't, you're fine. Or at least, it's someone else's fault if it does... – Coelostat

Hmm, I'm not sure about null pointers. The text assumes that an object is being deleted - this is not the case if you pass a null pointer. So it might still be undefined behavior because of a lack of saying what happens with null pointers. However it has the same lack in other texts that clearly shouldn't yield to UB. So I'm undecided. – Farrah

@Johannes: I didn't mean both things at once, I was just comparing, void foo(char*); foo(0);. This isn't "unconditional UB" (sharptooth's terminology) just because foo might be defined as void foo(char*p) {*p;}. Likewise, deleting through an incomplete type isn't "unconditional UB" just because the type might be completed with a non-trivial destructor. – Coelostat

@Steve I understand. It wasn't meant to be a reference to your comment :) I think I should more often include warnings such as "not related to statement of XYZ" since I find people make more associations than I would expect. My bad. – Farrah

@Johannes: Oh good. I think 5.3.5/2 covers null pointers in a way that isn't/can't be contradicted by 5.3.5/5. There is no "object being deleted", because the operation "has no effect" and so in particular it doesn't delete an object. Hence the "if" is false. – Coelostat

@Steve well it doesn't say "If an object being deleted has ..." but it says "If the object being deleted has ...". If it would say the former, I could see it doesn't apply to null pointers. 5.3.5/2 saying that deleting a null pointer "has no effect" doesn't really mean much, since an implementation is still allowed to call an deallocation function with a null pointer argument. I guess it's fine to delete null pointers though, but I really wouldn't want to bet. – Farrah

@Johannes: Fair point, but I don't see how 5.3.5/5 can conjure into existence the referand of a null pointer ;-) /2 also says, "In the second alternative (delete array), the value of the operand of delete shall be the pointer value which resulted from a previous array new-expression. If not, the behavior is undefined.". I think it's pretty clear that this is not meant to contradict the earlier statement that a null pointer is defined as a no-op, and the same can be said for the rest of the description of delete expressions. – Coelostat

@Steve I agree with that. However, C++0x's text changed here. At /2 it now says explicitly "If it is not a null pointer, in the first alternative ...", and /6 says explicitly "If the value ... is not a null pointer", while /5 still doesn't say so. So the intent seems to be towards UBness, but then /3 also omits it, so I've no clue what they want it to be :) – Farrah

@Johannes: interesting. I wonder whether they decided to omit it (and hence AFAIK introduce for the first time the concept of a null pointer referring to "an object"), or just added some tightening language to avoid any possibility of a contradiction in those other clauses, and didn't think it worth mentioning in /5. In /3, I don't think null pointers have a dynamic type, so just like "the object being deleted", the phrase "its dynamic type" is talking about something that doesn't exist. Should we assume that, like NaN, non-existent things don't compare equal to anything? ;-) – Coelostat

@Steve open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#348 uncovers that the change has nothing to do with our concerns -.- – Farrah

Also 12.5/4 that actually defines what deallocation function is called has no handling of null pointers. So it appears to be unclear what happens for a null pointer. However it appears that the last sentence of it applies: "If this lookup fails to find the name". That sentence will definitely apply for any T that isn't of class type. Maybe it is also intended to apply for null pointers. :) – Farrah

Answer 2

It is undefined behavior.

However, you can make the compiler check for incomplete types, like boost:

// verify that types are complete for increased safety

template<class T> inline void checked_delete(T * x)
{
    // intentionally complex - simplification causes regressions
    typedef char type_must_be_complete[ sizeof(T)? 1: -1 ];
    (void) sizeof(type_must_be_complete);
    delete x;
}

Applying sizeof to an incomplete type should trigger an error, and I suppose if that passes with some compiler, then an array of negative size would trigger an error.

Catharina

Answer 3

It is undefined behaviour, and a common gotcha when implementing the pImpl pattern. To the best of my knowledge, there is simply no such thing as a warning that the compiler is required to emit. Warnings are elective; they're there because the compiler writer thought they would be useful.

Emiliaemiliaromagna