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 <gga...@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 <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 _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
