Following up on C++11 final and undefined-reference linker errors

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-November/018779.html

A week ago I asked if adding |final| to a class causing linker errors could possibly not be a compiler bug; replies suggested it likely would be one. Having reduced it to the code at the end of this mail, I'm pretty sure it's not one. [expr.reinterpret.cast]p7 says reinterpret_cast<> on a class with virtual functions, or to a class which is not an actual type of the instance being cast, has undefined behavior. If I read this right, failure to link is permissible behavior. (GCC 4.7 also produces a linking error.)

That said, behavior here seems at least suboptimal. If the testcase shouldn't work, the error should be reported at compile time, when the devirtualizable call to a pure-virtual function is observed. Or maybe it "should" "work" and just do a virtual call, as it is in MSVC -- not in a correctness sense, but in a this-would-be-useful-if-it-worked-this-way sense.

Originally Wrapper was a templatized class which hid incref/decref methods on a reference-counting smart pointer class (the smart pointer's operator-> returns a Wrapper<T>*), prohibiting manual refcounting operations. (The testcase demonstrates the hiding trick for the 'hidden' virtual function.) It would be nice to be able to mark Wrapper as final to prohibit impermissible over-use of it, although it's not essential.

So: should anything be done here? And if yes, should it be to produce a compile error earlier, or to make this "work"?

Jeff

/*************************************************************************************/
#include <stdio.h>

#ifdef _MSC_VER
#define final sealed
#endif

class Base
{
   public:
     virtual void hidden() = 0;
     virtual void exposed() = 0;
     virtual ~Base() { }
};

class Derived : public Base
{
   public:
     virtual void hidden() { }
     virtual void exposed() { printf("exposed\n"); }
};

class Wrapper final : public Base
{
   private:
     virtual void hidden();
};

int main()
{
   Base* b = new Derived();

   // "should" print "exposed", undefined reference to Base::exposed() now
   reinterpret_cast<Wrapper*>(b)->exposed();
}

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-November/018779.html

A week ago I asked if adding |final| to a class causing linker errors could possibly not be a compiler bug; replies suggested it likely would be one. Having reduced it to the code at the end of this mail, I'm pretty sure it's not one. [expr.reinterpret.cast]p7 says reinterpret_cast<> on a class with virtual functions, or to a class which is not an actual type of the instance being cast, has undefined behavior. If I read this right, failure to link is permissible behavior. (GCC 4.7 also produces a linking error.)

Only if the undefined behavior is provably reached. Undefined behavior only taints execution paths which reach the undefined behavior.

Your program, however, is outright ill-formed, with no diagnostic required, because it contains the declaration of a virtual function (Wrapper::hidden()), but nowhere in the program is there a definition for it. All virtual functions are considered "used" and therefore must be defined, even if their classes are never even mentioned elsewhere in the program.

That said, behavior here seems at least suboptimal. If the testcase shouldn't work, the error should be reported at compile time, when the devirtualizable call to a pure-virtual function is observed.

I was going to say that this cannot actually happen because final classes are not permitted to be abstract, but as far as I can tell, that's not actually in the standard. That seems like an oversight, though, and I'd prefer to aggressively diagnose that with an error than to try to detect devirtualizable calls to pure virtual functions.

Regardless, your program does not actually contain a devirtualizable call to a pure virtual function, because Wrapper::hidden() is not pure virtual; it's merely nowhere defined.

Originally Wrapper was a templatized class which hid incref/decref methods on a reference-counting smart pointer class (the smart pointer's operator-> returns a Wrapper<T>*), prohibiting manual refcounting operations.

I would suggest making the refcounting operations private and befriending the smart pointer class.

John.

hidden() isn't really relevant here: we end up with the same behavior
even if you completely remove it from the testcase.

-Eli

You're right; I've misread the testcase.

I am still totally comfortable with unconditionally rejecting the definition of a final abstract class, however. Similarly, we should reject final pure virtual functions, although the error there is much more obvious and thus less likely to occur in practice.

John.

I agree that this is the best behavior. It makes absolutely zero sense having such a beast. We should also file a DR for that. Other languages (Java, C#) will complain about final classes with abstract members.

Sebastian

I am still totally comfortable with unconditionally rejecting the definition of a final abstract class, however.

I agree that this is the best behavior.

Thirded. Error.

It makes absolutely zero sense

having such a beast. We should also file a DR for that.

As Richard and I discussed this, we came to the same conclusion. He may have already started the process of filing a DR?

Yes, the process has begun.

Best wishes,
Richard