[webkit-dev] …Inlines.h vs …InlineMethods.h

[Darin Adler]

On Nov 5, 2012, at 8:47 AM, Geoffrey Garen <ggaren at apple.com> wrote:

> (2) Adopt the convention that any class using "*Inlines.h" defines all inline functions defined out-of-line in "*Inlines.h"

This is a proposal I don’t agree with.

Making a firm rule here here means that every larger inline function in one of these classes becomes a special case from a client point of view, where previously only functions actually affected by complex dependencies were special cases.

The proposed design requires adding a FooInlines.h include to source files that use that function, when any function moves into FooInlines.h. This can happen any time a function is made inline or when a short inline function gets longer. This could affect any file that was using that function but was not previously using one of the other functions in FooInlines.h. That is a burden I would prefer the project not take on; it would make refactoring more difficult.

Further, this proposal does not solve the problem of getting this wrong if we don’t actually try an appropriate build. As with today’s similar problems, include mistakes won’t be noticed if we are compiling with inlining off as we do on, say, Mac debug builds.

> Choosing what to put in FooInlines.h based on header dependencies hurts my brain.

In the name of making it easier to write the headers correctly and slightly easier to find functions, this will make it much more common to have to include a separate header file, without a clear rule for when you need that include.

The rule today is that we can just include the class’s header and use functions from that header, with a limited number of exceptions where we have to include another file. Putting more functions into separate files will spread this to other functions, making the problems with it worse.

> I don't want to compute the set of all header dependencies when trying to find a function definition.

We should not compute header dependencies in cases like this.

Instead we should look in all the source files, Foo.h, FooInlines.h, and Foo.cpp. Just as today we have to look in both Foo.h and Foo.cpp since we don’t know whether the function is inlined or not.

> Also, I don't want to move things around when dependencies change.


It would be good if we can accommodate you. But I don’t want to have to change all call sites when the complexity or inlining status of a member function changes.

The rest of your proposal is something I agree on.

-- Darin