Clang work

January 20, 2011

Googlers get 20% time — time to work on something not necessarily part of your direct goals — as an outlet to allow bottom-up work. I spent my 20% time over the last few months getting our build working under the clang compiler.

The work was kind of repetitive: adjusting our build system to support clang, then seeing what built, what didn't, and whether to blame our code or the compiler. It found a number of Chrome bugs! A simple check that gcc apparently lacked is accidentally initializing a member variable with itself rather than the argument:

class Foo {
  int bar_;
  Foo(int bar) : bar_(bar_) { }
  // Intended to write:
  // Foo(int bar) : bar_(bar) { }
};

We also found a number of clang bugs; here's a link to the bugs I'm involved with on their tracker, most of which are about bugs we encountered. It turned out that v8 in particular has a ton of gnarly C++ stuff related to instantiating templates. I found the delta tool particularly useful in providing reduced test cases, and my coworker Jeffrey Yasskin particularly helpful in C++ wizardry.

An aside: I'd provide a link for Jeffrey but everything I could find was pretty out of date; instead, a story I like to tell about interviewing. I once observed an interview where the interviewer asked the candidate to evaluate their knowledge on a scale from 1 to 5 in various areas on their resume, as a way of guiding which sorts of things they would ask. The candidate rated themselves a 4 on C++, which I now know in retrospect is actually a red flag: pretty much anyone who claims to be above a 3 out of 5 in C++ knows too little to know how much of C++ they don't know.

Jeffrey may be one of the rare true 4s; he patiently tried to explain to me argument-dependent and two-phase lookup a number of times. I doubt I would've been able to complete this clang project if he hadn't happened to be sitting in our office for those few months.

Now that Chrome builds under clang, I'm not exactly sure why I started the project. I think I was motivated by a few factors:

On that last point, two more stories.

One: a somewhat common code modification problem relates to the way overloading works. Suppose you have a base class with a virtual function and a tree of classes that sometimes imlement the interface.

class Base {
  virtual Foo* MakeFoo(int some_param) {
    return new SimpleFoo(some_param);
  }
};
class Derived : public Base {
  virtual Foo* MakeFoo(int some_param) {
    return new MoreComplicatedFoo(some_param);
  }
};

Now suppose you want to modify the signature of MakeFoo to take another argument. You need to adjust every class's MakeFoo, and if you miss any one of them, like Derived::MakeFoo, everything will compile just fine (C++ just sees it as a different overloaded function due to its different signature) but when you call MakeFoo on a Derived you'll get the wrong type back.

I lost a few hours to this once, and again was saved by a brilliant coworker who happened to be temporarily sitting in my office — this one immediately dereferenced the vtable on the pointer in gdb to see that we were getting back a SimpleFoo where we expected a MoreComplicatedFoo (as for why gdb couldn't tell us the type of the pointee itself, I don't know why it didn't work; the code was of course much more complicated than the above).

Anyway, it turns out that Microsoft's C++ compiler, I think due to C# making use of it, adds an override keyword that says "give me a compiler warning if this is not overriding a virtual function in a superclass". This would've saved me a lot of grief, had the functions been annotated. And it turns out that C++0x has some notion of the same thing, and that because of this (I think?) clang already parses __attribute__((override)) in the same location as Microsoft's keyword.

At the time I discovered this, the attribute was parsed but ignored. This allowed me to write a clang plugin to verify the overrides on Mac and Linux while relying on the system compiler on Windows. Here's the macro from the Chrome tree:

// Annotate a virtual method indicating it must be overriding a virtual
// method in the parent class.
// Use like:
//   virtual void foo() OVERRIDE;
#if defined(COMPILER_MSVC)
#define OVERRIDE override
#elif defined(__clang__)
#define OVERRIDE __attribute__((override))
#else
#define OVERRIDE
#endif

More recently the checking was added to clang itself (I should've just written it as part of the compiler in the first place, but I wanted to learn about plugins), so we now can use this throughout the Chrome code to hopefully make us more resilient to future refactoring bugs.

Here's the second clang story. A hard-to-spot typo made it into the Chrome tree; here's the diff, but the wrong code is easy enough to spot once you know something is wrong:

if (monitor |= currently_monitored) {

Nico from the Chrome team reasonably pointed out that you get a compiler warning when you use a single equals in an if, and filed a bug against clang. Hans from the Chrome team implemented it for clang. And after it landed, it found the same bug in the clang code itself!

There are more stories, like how Elliot has been writing analyzers to enforce some of our coding guidelines, but this post is long enough already. For more information, see our wiki page which links to our mailing list and other projects.