16.11.11

Overriding inherited virtual functions

One way C++ supports code reuse is through inheritance. One base class implements common functionality. Then other classes inherit from it, essentially copying functionality from it. These other classes can add their own new functionality, or, more powerfully, they can override the base class functionality.

Overriding base class functionality is simple. Keeping such overrides working correctly is sometimes harder. The problem is that the override relationship is implicit: if the override doesn’t exactly match the signature of the desired function in the base class, it may not work correctly.

Making the override relationship explicit

Some languages (Scala, C#, probably others) provide the ability to mark a derived class’s function as an override of an inherited function. C++98 included no such ability, but C++11 does, through the contextual override keyword. When override is used, that virtual member function must override one found on a base class. If it does not, it is a compile error.

Introducing MOZ_OVERRIDE

The Mozilla Framework Based on Templates now includes support for the C++11 contextual override keyword, encapsulated in the MOZ_OVERRIDE macro in mozilla/Types.hmozilla/Attributes.h. Simply place it at the end of the declaration of the relevant method, before any = 0 or method body, like so:

MOZ_OVERRIDE will expand to use the C++11 construct in compilers which support it. Thus in such compilers misuse of MOZ_OVERRIDE is an error. Even better, some of the compilers used by tinderbox support override, so in many cases tinderbox will detect misuse. (Specifically, MSVC++ 2005 and later support it, so errors in cross-platform and Windows code won’t pass tinderbox . Much more recent versions of GCC and Clang support it as well, but these versions are too new for tinderbox to have picked them up yet — in the case of GCC too new to even have been released yet. 🙂 )

What about NS_OVERRIDE?

It turns out there’s already a macro annotation to indicate an override relationship: NS_OVERRIDE. This gunky XPCOM macro expands to a user attribute under gcc-like compilers. It’s only used by static analysis right now, so its value is limited. Unfortunately its position is different — necessarily so, because in the C++11 override position it would attach to the return value of the method:

NS_OVERRIDE is now deprecated and should be replaced with MOZ_OVERRIDE. With a little work, static analysis with new-enough compilers can likely look for MOZ_OVERRIDE just as easily as for NS_OVERRIDE. And since MOZ_OVERRIDE works in non-static analysis builds, it’s arguably better in the majority of cases anyway. If you’re looking for an easy way to improve Mozilla code, changing NS_OVERRIDE uses to use MOZ_OVERRIDE would be a simple way to help.

Summary

If you’ve overridden an inherited virtual member function and you’re worried that that override might silently break at some point, annotate your override with MOZ_OVERRIDE. This will cause some compilers to enforce an override relationship, making it much less likely that your intended relationship will break.

C++ default operators and the sole-ownership idiom

Often a C++ class will solely manage some value: for example, a GtkWindow* or a void* for malloc‘d memory. The class will then release ownership in its destructor as appropriate. It would be extremely problematic to release ownership multiple times — think security-vulnerability-problematic. C++ copy construction and default assignment exacerbate this issue, because C++ automatically generates these methods for all classes, even when the default behavior breaks sole-ownership. The C++98 idiom solving this is to privately declare a copy constructor and a default assignment operator, then never define them:

Declaring the methods privately prevents any code but friends of Struct from calling them. And by never defining them, even such friends will cause a link-time error if they try.

Disabling the default operators in C++11

Once you’re familiar with this idiom it’s not too bad. But initially, it’s pretty unclear. And nothing prevents someone from actually defining these methods. (They could only be used by Struct or friends of Struct, to be sure, but for sufficiently complex code it’s possible someone might make a mistake.) C++11 improves upon this trick by introducing deleted function syntax:

MOZ_DELETE isn’t as readable or understandable as = delete, but it’s searchable, and the comment next to its definition will clarify matters. If the declarations are private, MOZ_DELETE is just as good as the traditional idiom, and in compilers supporting C++11 deleted functions it’s better.

Which compilers support C++11 deleted functions? I’m aware of GCCsince 4.4, Clang since 2.9, and ICCsince 12.0. Rightly, if unfortunately, you must specify -std=c++0x or similar to use deleted function syntax without causing a warning. For various reasons Mozilla can’t do that yet, so MOZ_DELETE only produces the C++11 syntax when compiling with Clang (where we can pass -Wno-c++0x-extensions to disable the warning). I’d love to see it use C++11 syntax in GCC and ICC as well, but I don’t have the time to solve the -std=c++0x problem now, or to figure out another workaround. I’ve filed bug 701183 for this problem — help there is much appreciated.

Summary

Use MOZ_DELETE when declaring any method you will intentionally not implement. It’ll work better, and produce better errors, in some compilers. Those compilers don’t include GCC or ICC yet, but with your help they could. Any takers?

Update, evening of November 10, 2011: I just landed further changes to make MOZ_DELETE use C++11 syntax with GCC when compiling with -std=c++0x (which we apparently do more often than I’d thought), so you should now get its goodness in GCC as well — most of the time. In some “exotic” situations we don’t compile anything with -std=c++0x, so you won’t get any benefit there. Also, the JavaScript engine is never compiled with it. So if you want this to work fully, everywhere, you should use Clang.

20.10.11

In my last post I announced the addition of mozilla::ArrayLength and mozilla::ArrayEnd to the Mozilla Framework Based on Templates, and I noted I was leaving a description of how these methods were implemented to a followup post. This is that post.

The trick is this: you can templatize an array based on its compile-time length. Here we templatize both methods on: the type of the elements of the array, so that each is polymorphic; and the number of elements in the array. Then inside the method we can refer to that length, a constant known at compile time, and simply return it to implement the desired semantics.

Templatizing on the length of an array may not seem too unusual. The part that may be a little unfamiliar is how the array is described as a parameter of the template method: T (&arr)[N]. This declares the argument to be a reference to an array of N elements of type T. Its being a reference is important: we don’t actually care about the array contents at all, and we don’t want to copy them to call the method. All we care about is its type, which we can capture without further cost using a reference.

This technique is uncommon, but it’s not new to Mozilla. Perhaps you’ve wondered at some point why Mozilla’s string classes have both EqualsLiteral and EqualsASCII: the former for use only when comparing to “an actual literal string”, the latter for use when comparing to any const char*. You can probably guess why this interface occurs now: EqualsLiteral is a method templatized on the length of the actual literal string passed to it. It can be more efficient than EqualsASCII because it knows the length of the compared string.

Using this trick in other code

I did most of the work to convert NS_ARRAY_LENGTH to ArrayLength with a script. But I still had to look over the results of the script to make sure things were sane before proceeding with it. In doing so, I noticed a decent number of places where an array was created, then it and its length were being passed as arguments to another method. For example:

Using ArrayLength here is safer than hard-coding a length. But safer still would be to not require callers to pass an array and a length separately — rather to pass them together. We can do this by pushing the template trick down another level, into NS_RegisterStaticAtoms (or at least into an internal method used everywhere, if the external API must be preserved for some reason):

The pointer-and-length method generally still needs to stick around somewhere, and it does in this rewrite here. It’s just that it wouldn’t be the interface user code would see, or it wouldn’t be the primary interface (for example, it might be protected or similar).

NS_RegisterStaticAtoms was just one such method which could use improvement. In a quick skim I also see:

…as potential spots that could be improved — at least for some callers — with some additional templatization on array length.

I didn’t look super-closely at these, so I might have missed some. Or I might have been over-generous in what could be rewritten to templatize on length, seeing only the one or two places that pass fixed lengths and missing the majority of cases that don’t. But there’s definitely a lot of cleaning that could be done here.

A call for help

Passing arrays and lengths separately is dangerous if you don’t know what you’re doing. The trick used here eliminates it, in certain cases. The more we can use this pattern, the more we can fundamentally reduce the danger of separating data from its length.

I don’t have time to do the above work myself. (I barely had time to do the ArrayLength work, really. I only did it because I’d sort of started the ball rolling in a separate bug, so I felt some obligation to make sure it got done.) And it’s not particularly hard work, requiring especial knowledge of the relevant code. It’s a better use of my time for me to work on JavaScript engine code, or on other code I know particularly well, than to do this work. But for someone interested in getting started working on Gecko C++ code, it would be a good first project. I’ve filed bug 696242 for this task; if anyone’s interested in a good first bug for getting into Mozilla C++ coding, feel free to start with that one. If you have questions about what to do, in any way, feel free to ask them there.

On the other hand, if you have any questions about the technique in question, or the way it’s used in Mozilla, feel free to ask them here. But if you want to contribute to fixing the issues I’ve noted, let’s keep them to the bug, if possible.