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

[Adam Barth]

Just so I understand, you're recommending we move functions like
Document::guardDeref()

http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.h#L240

out of the class declaration, but leave them in the header file (e.g.,
in Document.h).

Thanks!
Adam


On Mon, Nov 5, 2012 at 8:47 AM, Geoffrey Garen <ggaren at apple.com> wrote:
> 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 <darin at apple.com> wrote:
>
>> On Nov 3, 2012, at 10:09 AM, Mark Lam <mark.lam at 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 at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev