Based on the feedback here, I would propose something like the following:
(1) Globally rename "*InlineMethods.h" => "*Inlines.h".
"*Inlines.h" is now the WebKit convention for how you put an inline function in
a separate file.
Update the style guide to say so.
(2) Adopt the convention that any class using "*Inlines.h" defines all inline
functions defined out-of-line in "*Inlines.h"
Choosing what to put in FooInlines.h based on header dependencies hurts my
brain. I don't want to compute the set of all header dependencies when trying
to find a function definition. Also, I don't want to move things around when
dependencies change.
(3) Refactor to use "*Inlines.h" in the few cases in JSC where functions are in
obviously unholy places.
(4) Refactor to use "*Inlines.h" in the few cases where it would substantially
improve compile times. (Do we know of such cases?)
(5) Adopt the convention that any function that is not as trivial as "int x() {
return m_x; }" moves out of the class declaration.
This makes finding functions easier in the world of "*Inlines.h". Also, this
puts class interface and data first, making things more readable.
Update the style guide to say that "int x() { return m_x; }" should be one line.
Update the style guide to say that more complex functions should not reside in
the class declaration.
How does that sound?
Geoff
On Nov 3, 2012, at 4:04 PM, Darin Adler <da...@apple.com> wrote:
> On Nov 3, 2012, at 10:09 AM, Mark Lam <mark....@apple.com> wrote:
>
>> 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.
>
> Inline functions that are kept inside vs. outside class definitions is a
> separate issue from the in the same header file vs. in another header file.
>
> I agree that class definitions are often significantly easier to read if all
> non-trivial function definitions are put outside the class. But requiring
> that all such function definitions be put into a separate file seems unduly
> awkward and heavy to me.
>
> Maciej stated the reasons we have created such files in the past; the intent
> was not to put all inline functions in them. I would not want to create the
> many new files this new convention would require.
>
>> 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;
>> }
>
> The WebKit coding style document might formally require that as currently
> written, but I think it’s a mistake and not consistent with the style we
> actually use in WebKit coding.
>
>> I would propose updating the webkit coding style to allowing braces on the
>> same line for the case of 1 line inline functions
>
> We should. It’s already part of WebKit coding style, even though it seems not
> yet part of the formal WebKit coding style document.
>
> -- Darin
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
