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

[Mark Lam]

On Nov 3, 2012, at 2:11 AM, Maciej Stachowiak <mjs at apple.com> wrote:
> On Nov 3, 2012, at 1:48 AM, Mark Lam <mark.lam at apple.com> wrote:
>> 1. If the inline method is a 1 liner, then leave it in the original header file.
>> 2. If it takes more than 1 line, then move it into a corresponding inline header file.
> 
> What is the intended benefit of more thoroughly splitting out inline methods? The two reasons I know to do it are:
> 
> (a) In some cases, it is required to avoid a circular dependency in header includes (where two classes have inline member functions that each depend on the other class).
> (b) If inline methods require pulling in additional includes, but not all files including the main header need their definitions, it can speed up compile times.
> 
> It does not seem to me like a cutoff of 1 line vs more than 1 line is in very good alignment with reasons (a) and (b) to split inline member functions into a separate header.
> 
> So I'd like to understand why we want to do this.


Avoiding circular dependency issues is the reason why I looked into inline headers in the first place.  Currently, there are inline functions that are put in different header files (not an intuitive place and requires a grep to find) just to avoid the circular dependency.  I would like to make it more intuitive to know where to find functions without having to grep i.e. would like to have a cleaner model of how source code is packaged (and present less mental clutter for the engineer working with the code).  

My reasons for proposing the 1 line vs multi-line convention are:

1. to keep class definitions more compact
    - so you can see more of the entire class (not that this is always possible anyway).
    - being able to see the entire class (when possible) can yield useful information about the shape of the class i.e. let's me see the architecture / design, not just the implementation.
    - having lots of inline functions bodies inside the class definition bloats the class significantly and makes it harder to see the shape. (again, a mental clutter issue).  Of course, if you have editor tools that can hide the function bodies, this is not an issue.

2. to provide some consistency to the styling as to where to expect to find inline functions (i.e. inline methods are in Inlines.h).

However, it would be undue burden to have to move all trivial inline functions like 1 line setter/getters to an Inlines.h file.  By 1 liners, I mean something like:

     bool isFinished { return m_isFinished; }

… but now am realizing that this is not allowed by the webkit coding style, which instead requires:

     bool isFinished
     {
         return m_isFinished;
     } 

Anyway, I proposed this 1 liner convention (with assuming it is really a 1 liner, not a 4 liner) so that we don't have to move trivial inlines to the inline header.  Having a mechanical convention for this also makes it so that the engineer will not have to make a value judgement on the compactness issue when writing the code and can focus on the implementation (again less mental clutter).

> The main downside is that you now have to keep track of whether you need the second header when you use a class, and if you forget, you may not see the failure until late in the build process.

Agreed.  It's unfortunate but true.

> It also seems like it would create friction when changing inline methods, since taking it over or under the one-line cutoff would now require you to move it to a different file.

Also agreed, but it would prevent slop which eventually leads to the class definition bloating up … which is what I was trying to avoid in the first place.

As for Filip's argument:

> The general challenge here is that many of the methods have semantics that are best deduced by not only reading their names but also looking at their contents.

I'm not sure I agree with this point as much.  I agree with the "sometimes function names are not adequate" part, but wouldn't it be equally meaningful if you were reading effectively the same body of code (the inline functions) in the Inlines.h file instead?  Am I not seeing something here?

That said, since the webkit coding style prohibits true 1 line inline functions, I feel less enthusiastic about my proposal as it would either:
1. automatically necessitate moving a lot of functions (all the 1 liners that are expressed as 4 liners), or
2. not give me the benefit I want (i.e. to improve my chances of seeing the entire class at a glance) because lots of 4 liners will still bloat the class definition. 

I would propose updating the webkit coding style to allowing braces on the same line for the case of 1 line inline functions because that is a good thing (code compactness), but that is a separate subject altogether.  As for my 1 line vs multi-line convention, for now, I'm inclined to go with dropping it, and leaving the decision to move a function to Inlines.h or not to the discretion of the engineer working on the code.

Any opinions / thoughts?  Thanks.

Regards,
Mark