http://codereview.chromium.org/6541044/diff/4003/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/6541044/diff/4003/src/heap.h#newcode2184
src/heap.h:2184: void VisitPointers(Object** start, Object** end) {
On 2011/02/22 03:11:15, marklam wrote:
On 2011/02/21 18:08:28, Mikhail Naganov (Chromium) wrote:
> Does this method need to be inline?

Done.  This is a virtual method override (and I think the compiler
will make
sure to only instantiate one instance of it).  The pattern of
specifying this
inline is used in lots of places from what I can tell.  I didn't know
if that
was the preferred expression.  Anyway, I've moved it to the cc file
now.

I'm just against having too much code in .h file. It is preferred to
leave only getters and setters there. If some non-trivial code really
needs to be inline, we usually have -inl.h file for it.

http://codereview.chromium.org/6541044/diff/4003/src/heap.h#newcode2203
src/heap.h:2203: protected:
On 2011/02/22 03:11:15, marklam wrote:
On 2011/02/21 18:08:28, Mikhail Naganov (Chromium) wrote:
> I'd prefer to use 'private:' in all cases, except when you really
design your
> class to be subclassed.

I do intend for this class to be subclassed.  That will come in the
LOL code
(which is what prompted this refactoring in the first place).  This is
also why
ProcessResults() is virtual.


OK, I just wanted to be sure. Sorry, I forgot the whole picture.

Or do you mean that you want some members to be limited to being
private instead
of protected?  Currently, I don't think the LOL implementation needs
access to
all of these members, but it seems extremely limiting to assume what
the
subclass needs and not need in the future.  Unless you object to it
being
somewhat general like this for forward looking purposes, I would
prefer to keep
all these members protected instead of private.

That's OK.

http://codereview.chromium.org/6541044/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to